feat(other): add basic BankAccount OOP class#14543
feat(other): add basic BankAccount OOP class#14543nickzerjeski wants to merge 5 commits intoTheAlgorithms:masterfrom
Conversation
Closing this pull request as invalid@nickzerjeski, this pull request is being closed as none of the checkboxes have been marked. It is important that you go through the checklist and mark the ones relevant to this pull request. Please read the Contributing guidelines. If you're facing any problem on how to mark a checkbox, please read the following instructions:
NOTE: Only |
There was a problem hiding this comment.
Pull request overview
This PR introduces a simple BankAccount OOP example with doctests, but it also adds two additional, unrelated algorithm modules (repunit theorem utilities and a space-optimized knapsack implementation).
Changes:
- Added
other/bank_account.pyimplementing a basicBankAccountwith deposit/withdraw and doctests. - Added
maths/repunit_theorem.pywith repunit divisibility helpers and doctests. - Added
knapsack_space_optimized()todynamic_programming/knapsack.pywith validation and doctests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| other/bank_account.py | New BankAccount class with encapsulated account number and balance operations (doctested). |
| maths/repunit_theorem.py | New repunit theorem utilities (has_repunit_multiple, least_repunit_length) with doctests. |
| dynamic_programming/knapsack.py | Adds a 1D DP, space-optimized 0/1 knapsack implementation with input validation and doctests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| """ | ||
| Utilities related to repunits and a classical repunit divisibility theorem. | ||
|
|
||
| A repunit of length ``k`` is the number made of ``k`` ones: | ||
| ``R_k = 11...1``. | ||
|
|
||
| For every positive integer ``n`` with ``gcd(n, 10) = 1``, | ||
| there exists a repunit ``R_k`` divisible by ``n``. | ||
| """ |
There was a problem hiding this comment.
PR description/title focus on adding BankAccount, but this change set also adds a new maths module (maths/repunit_theorem.py) and a new knapsack implementation. Please either update the PR description/title to reflect the additional features or split these into separate PRs to keep the change scoped and reviewable.
| def __init__(self, account_number: str, initial_balance: float = 0.0) -> None: | ||
| if not account_number: | ||
| raise ValueError("account_number must be provided") | ||
| if initial_balance < 0: | ||
| raise ValueError("initial_balance cannot be negative") | ||
|
|
||
| self.__account_number = account_number | ||
| self._balance = float(initial_balance) |
There was a problem hiding this comment.
initial_balance < 0 will not reject NaN (comparisons with NaN are always false), so BankAccount(..., initial_balance=float('nan')) creates an account with a corrupted balance. Consider coercing initial_balance to float first and rejecting non-finite values (NaN/inf) before storing it.
| def deposit(self, amount: float) -> float: | ||
| """Deposit a positive amount and return updated balance.""" | ||
| if amount <= 0: | ||
| raise ValueError("deposit amount must be positive") | ||
| self._balance += amount | ||
| return self._balance | ||
|
|
||
| def withdraw(self, amount: float) -> float: | ||
| """Withdraw a positive amount and return updated balance.""" | ||
| if amount <= 0: | ||
| raise ValueError("withdraw amount must be positive") | ||
| if amount > self._balance: | ||
| raise ValueError("insufficient funds") | ||
| self._balance -= amount | ||
| return self._balance |
There was a problem hiding this comment.
amount <= 0 does not reject NaN, and adding/subtracting NaN will propagate and permanently corrupt the balance. Consider validating that amount is a finite number (and then > 0) before applying the operation (same for both deposit and withdraw).
Description
Add a simple OOP
BankAccountimplementation with encapsulated account number and safe balance operations.Fixes #14024
Changes
other/bank_account.pywith:account_numberbalancepropertydeposit/withdrawmethods with validationValidation
python3 -m doctest -v other/bank_account.py