Skip to content

feat(other): add basic BankAccount OOP class#14543

Closed
nickzerjeski wants to merge 5 commits intoTheAlgorithms:masterfrom
nickzerjeski:feat-bank-account-class-14024
Closed

feat(other): add basic BankAccount OOP class#14543
nickzerjeski wants to merge 5 commits intoTheAlgorithms:masterfrom
nickzerjeski:feat-bank-account-class-14024

Conversation

@nickzerjeski
Copy link
Copy Markdown

Description

Add a simple OOP BankAccount implementation with encapsulated account number and safe balance operations.

Fixes #14024

Changes

  • Added other/bank_account.py with:
    • read-only account_number
    • balance property
    • deposit / withdraw methods with validation
  • Added doctests for the main flow.

Validation

  • python3 -m doctest -v other/bank_account.py

Copilot AI review requested due to automatic review settings April 13, 2026 08:44
@algorithms-keeper
Copy link
Copy Markdown

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:

  • Read a point one at a time and think if it is relevant to the pull request or not.
  • If it is, then mark it by putting a x between the square bracket like so: [x]

NOTE: Only [x] is supported so if you have put any other letter or symbol between the brackets, that will be marked as invalid. If that is the case then please open a new pull request with the appropriate changes.

@algorithms-keeper algorithms-keeper bot added awaiting reviews This PR is ready to be reviewed and removed awaiting reviews This PR is ready to be reviewed labels Apr 13, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.py implementing a basic BankAccount with deposit/withdraw and doctests.
  • Added maths/repunit_theorem.py with repunit divisibility helpers and doctests.
  • Added knapsack_space_optimized() to dynamic_programming/knapsack.py with 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.

Comment on lines +1 to +9
"""
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``.
"""
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +21 to +28
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)
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
Comment on lines +40 to +54
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
Copy link

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment