forked from AdaGold/BankAccounts
-
Notifications
You must be signed in to change notification settings - Fork 26
Bank Accounts! #11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
nahmisa
wants to merge
37
commits into
Ada-C5:master
Choose a base branch
from
nahmisa:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Bank Accounts! #11
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
Account class accept a hash. I currently need an ID and a beginning balance. Def withdraw; handles error of account potentially going negative.
Returns a collection of Account instances, representing all of the Accounts described in the CSV. CSV gives ID, Balance, and Open Date (of account)
Returns an instance of Account where the value of the id field in the CSV matches the passed parameter. *****Moved class methods to the end of the class for clarity.
…. what I wanted to do is still the same message but now it's a real thing that is the best.
self.all - returns a collection of Owner instances, representing all of the Owners described in the CSV. See below for the CSV file specifications self.find(id) - returns an instance of Owner where the value of the id field in the CSV matches the passed parameter Updated self methods to refer to class as self.XXXX instead of Module::Class.XXXX in my class methods
…ccounts To create the relationship between the accounts and the owners use an account_owners.csv file. The data for this file, in order in the CSV, consists of: 1. Account ID - (Fixnum) a unique identifier corresponding to an Account instance. 1. Owner ID - (Fixnum) a unique identifier corresponding to an Owner instance. get_owners_accounts_ids uses the CSV file to return an array of all account ids associated with the owner instance's owner id. return_owners_accounts translates these account ids into an array of account instances (with the matching account ids) using the class method Account.find(id)
…converted to fixnum (not float)
In all self.all and self.find methods, and in "accounts" methods in owner class. Can still change data files if need be when calling these methods, but default is our CSV files for the assigment.
…tants instead of numbers) This lets me inherit methods like withdraw and deposit to sub-classes like SavingsAccount without modifying the method. Instead, I can update the CONSTANTS.
Checking account class draws on methods from Account (withdraw and deposit) with updated constants for minimum balance (0) and transaction fee (1). Added a method for check withdrawl. Checks can be overdraft up to $10 and fees varry based on the number of checks issued per month. This behavior is captured in a new method, withdraw_by_check. Other additions to the Account baseclass to make this instance method possible are the ability to count check (check count). Methods to update how many checks have been issued are contained within the subclass. Phew. Shoulda committed more frequently.
Updated withdrawl method to fulfil requirements for a MoneyMarket account (no more than 6 trx, balance minimum of 10,000, fine if the withdrawl brings balance below that amount)
…ause us to reach or exceed min. balance
Since both MoneyMarketAccounts and Savings accounts are using interest computations, I moved this to base class. When called on CheckingAccount, add_interest returns a message saying checking accounts don't get interest and does not calculate interest or update the balance.
Used arguments in each method instead of constants. Refactored out pieces of code from bigger methods (ie, accessing @balance to add/remove money) so I could reuse pieces accross subclasses. Withdraw and deposit use these methods (add_money, remove_money) and then what needs to change between classes is the text that is puts'ed to the screen so user messages are still relevant. Added methods evaluating conditionals so hopefully if/elsif/else is a little more human-digestible
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.
Individual project.
I'm pretty proud of the way I cleaned up this code between the version that used constants to account for the different account constraints (d61cfac) and my last commit. I know I should have committed more between the two versions, but I wasn't really banking on being successful and thought that the experiment using more arguments per method would be just that: an experiment.
So that's all! Advice on when to put things in private methods would be very much appreciated :) PRIVACY IS IMPORTANT AT BANKS!!!!