feat(algorithms) non constructible change #95
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Contributor
WalkthroughAdds a new "Non-Constructible Change" problem under Algorithms -> Arrays: documentation entry, problem README, a Python implementation exposing non_constructible_change(coins: List[int]) -> int, and a unittest suite validating multiple scenarios. Changes
Sequence Diagram(s)sequenceDiagram autonumber actor Caller participant Func as non_constructible_change Caller->>Func: call with coins: List[int] note over Func: make sorted copy\ncurrent_change_created = 0 loop for each coin in sorted coins alt coin > current_change_created + 1 Func-->>Caller: return current_change_created + 1 else note over Func: current_change_created += coin end end Func-->>Caller: return current_change_created + 1 Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks (1 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews: pre_merge_checks: custom_checks: - name: "Undocumented Breaking Changes" mode: "warning" instructions: | Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
algorithms/arrays/non_constructible_change/README.md (2)
12-20: Use a standard fence language identifier.“plain” isn’t commonly recognized; prefer “text” (or omit).
Apply this diff:
-```plain +```text coins = [5,7,1,1,2,3,22]@@
-plain +text
20
1-7: Add a brief approach and complexity section.Two short lines for method (greedy with sorted coins) and time/space (O(n log n), O(1) or O(n) if not mutating) will help readers.
algorithms/arrays/non_constructible_change/test_non_constructible_change.py (1)
90-99: Add a test asserting the input list isn’t mutated.Guards against accidental in-place sorting.
Apply this diff:
def test_13(self): """should return 29 for input of [1, 2, 3, 4, 5, 6, 7]""" coins = [1, 2, 3, 4, 5, 6, 7] expected = 29 actual = non_constructible_change(coins) self.assertEqual(actual, expected) + def test_input_not_mutated(self): + """should not mutate the input coins list""" + coins = [5, 7, 1, 1, 2, 3, 22] + snapshot = coins[:] + _ = non_constructible_change(coins) + self.assertEqual(coins, snapshot) + if __name__ == '__main__': unittest.main()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
DIRECTORY.md(1 hunks)algorithms/arrays/non_constructible_change/README.md(1 hunks)algorithms/arrays/non_constructible_change/__init__.py(1 hunks)algorithms/arrays/non_constructible_change/test_non_constructible_change.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
algorithms/arrays/non_constructible_change/test_non_constructible_change.py (1)
algorithms/arrays/non_constructible_change/__init__.py (1)
non_constructible_change(4-17)
🪛 markdownlint-cli2 (0.17.2)
DIRECTORY.md
12-12: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 4; Actual: 6
(MD007, ul-indent)
🔇 Additional comments (2)
algorithms/arrays/non_constructible_change/__init__.py (1)
4-17: Algorithm is correct and minimal.Greedy logic and boundary handling look good.
algorithms/arrays/non_constructible_change/test_non_constructible_change.py (1)
5-95: Good breadth of cases.Coverage across empties, singles, duplicates, and large gaps is solid.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
algorithms/arrays/non_constructible_change/test_non_constructible_change.py (2)
5-96: Reduce duplication with table-driven subTests.Same coverage, smaller surface area, easier to extend.
class NonConstructibleChangeTestCase(unittest.TestCase): - def test_1(self): - """should return 20 for input of [5, 7, 1, 1, 2, 3, 22]""" - coins = [5, 7, 1, 1, 2, 3, 22] - expected = 20 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_2(self): - """should return 6 for input of [1, 1, 1, 1, 1]""" - coins = [1, 1, 1, 1, 1] - expected = 6 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_3(self): - """should return 55 for input of [1, 5, 1, 1, 1, 10, 15, 20, 100]""" - coins = [1, 5, 1, 1, 1, 10, 15, 20, 100] - expected = 55 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_4(self): - """should return 3 for input of [6, 4, 5, 1, 1, 8, 9]""" - coins = [6, 4, 5, 1, 1, 8, 9] - expected = 3 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_5(self): - """should return 1 for input of []""" - coins = [] - expected = 1 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_6(self): - """should return 1 for input of [87]""" - coins = [87] - expected = 1 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_7(self): - """should return 32 for input of [5, 6, 1, 1, 2, 3, 4, 9]""" - coins = [5, 6, 1, 1, 2, 3, 4, 9] - expected = 32 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_8(self): - """should return 19 for input of [5, 6, 1, 1, 2, 3, 43]""" - coins = [5, 6, 1, 1, 2, 3, 43] - expected = 19 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_9(self): - """should return 3 for input of [1, 1]""" - coins = [1, 1] - expected = 3 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_10(self): - """should return 1 for input of [2]""" - coins = [2] - expected = 1 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_11(self): - """should return 2 for input of [1]""" - coins = [1] - expected = 2 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_12(self): - """should return 87 for input of [109, 2000, 8765, 19, 18, 17, 16, 8, 1, 1, 2, 4]""" - coins = [109, 2000, 8765, 19, 18, 17, 16, 8, 1, 1, 2, 4] - expected = 87 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) - - def test_13(self): - """should return 29 for input of [1, 2, 3, 4, 5, 6, 7]""" - coins = [1, 2, 3, 4, 5, 6, 7] - expected = 29 - actual = non_constructible_change(coins) - self.assertEqual(actual, expected) + def test_cases(self): + cases = [ + ([5, 7, 1, 1, 2, 3, 22], 20), + ([1, 1, 1, 1, 1], 6), + ([1, 5, 1, 1, 1, 10, 15, 20, 100], 55), + ([6, 4, 5, 1, 1, 8, 9], 3), + ([], 1), + ([87], 1), + ([5, 6, 1, 1, 2, 3, 4, 9], 32), + ([5, 6, 1, 1, 2, 3, 43], 19), + ([1, 1], 3), + ([2], 1), + ([1], 2), + ([109, 2000, 8765, 19, 18, 17, 16, 8, 1, 1, 2, 4], 87), + ([1, 2, 3, 4, 5, 6, 7], 29), + ] + for coins, expected in cases: + with self.subTest(coins=coins): + self.assertEqual(non_constructible_change(coins), expected)
5-95: Clarify input contract; consider tests for invalid coins.If zeros/negatives are invalid, add tests asserting ValueError (or document allowed domain). If allowed, add cases with 0 to ensure behavior remains correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
algorithms/arrays/non_constructible_change/README.md(1 hunks)algorithms/arrays/non_constructible_change/__init__.py(1 hunks)algorithms/arrays/non_constructible_change/test_non_constructible_change.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- algorithms/arrays/non_constructible_change/README.md
- algorithms/arrays/non_constructible_change/init.py
🧰 Additional context used
🧬 Code graph analysis (1)
algorithms/arrays/non_constructible_change/test_non_constructible_change.py (1)
algorithms/arrays/non_constructible_change/__init__.py (1)
non_constructible_change(4-28)
🔇 Additional comments (2)
algorithms/arrays/non_constructible_change/test_non_constructible_change.py (2)
97-103: Nice: explicit non-mutation test.This protects the contract around input immutability.
1-3: List import and usage verified; no changes needed.
| @@ -0,0 +1,105 @@ | |||
| import unittest | |||
| from . import non_constructible_change | |||
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use absolute import so tests work via discovery and when run as a script.
The relative import breaks if this file is executed directly (your main block implies that use). Prefer an absolute import.
-from . import non_constructible_change +from algorithms.arrays.non_constructible_change import non_constructible_change📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from . import non_constructible_change | |
| from algorithms.arrays.non_constructible_change import non_constructible_change |
🤖 Prompt for AI Agents
In algorithms/arrays/non_constructible_change/test_non_constructible_change.py around line 2, the test uses a relative import which fails when the file is run directly; change the import to an absolute import using the full package path (algorithms.arrays.non_constructible_change) so test discovery and direct execution both work; update the import statement accordingly and run the test file directly to verify it imports correctly. Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
Describe your change:
Non constructible change algorithm
Checklist:
Summary by CodeRabbit