Skip to content
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

Jackie - Bank Account #33

Open
wants to merge 42 commits into
base: master
Choose a base branch
from
Open

Conversation

jackiewatanabe
Copy link

Bank Account

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer
Why is it useful to put classes inside modules? What do you think? It's useful to put classes inside modules so that classes are better organized. For example, there are all different types of accounts besides Bank accounts (social media, OS login, etc), but creating a module Bank, specifies that this account will be dealing with financial information
What is accomplished with raise ArgumentError? What do you think it's doing? It flags an error to the programmer so they know they coded an incorrect argument for one of the parameters.
Why do you think we made the .all & .find methods class methods? Why not instance methods? because the all and find methods don't do anything to change any of the data in the created accounts. It's merely used as a reference tool.
Why does it make sense to use inheritance for the different types of accounts? Because savings and checking accounts have similar attributes as a general account (withdrawing, depositing, balances), but they also have more specific characteristics to those attributes/methods
Did the presence of automated tests change the way you thought about the problem? How? Yes...added a new element of frustration! Kidding... sort of. ;) But it did help me make sure my code was more specific to all kinds of situations. It also helped create a road map of how my code should be laid out.

…ro and also update the balance with the deposit amount
…ted test to make sure the number of accounts is correct
… accounts.rb to raise an error if there is an unexisting account passed in when using the .find class method
…id of some if/else statements by putting raise ArgumentError first
…unt.all so that the file is read only once when tested.
@kariabancroft
Copy link

Bank Account

What We're Looking For

Feature Feedback
Wave 1
All provided tests pass Yes
Using the appropriate attr_ for instance variables Yes
Wave 2
All stubbed tests are implemented fully and pass Yes - You are doing a before block in the find tests that I think is unnecessary. You are never using the @account_array instance variable in those tests.
Created instances (by calling new) in Account.all Yes
Used CSV library only in Account.all (not in Account.find) Yes
Used Account.all to get account list in Account.find Yes
Wave 3
Used inheritance in the initialize for both Checking and Savings Accounts (min balance) Not quite. You are duplicating the min balance logic in the initialize in the "regular" and savings account.
Used inheritance for the withdraw in checking and savings accounts Yes - utilizing the logic which will check the withdrawal amount and the actual withdrawal math operation.
Testing Nice job covering the major behavior. If you use a before block, you could clean up your CheckingAccount.new & SavingsAccount.new duplication. Your check tests could use an array with a loop to execute the withdraw_using_check method some number of times to clean up the duplication as well.
Baseline
Used Git Regularly Yes! Really nice job with the frequency of your commits
Answer comprehension questions Good job with the comprehension questions. I like the way you described tests created a "road map" for your code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants