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

Lynn Trickey's Bank Account #23

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

Lynn Trickey's Bank Account #23

wants to merge 74 commits into from

Conversation

ltrickey
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? Because then we know that all accounts act alike and are part of the Bank, and there's less work to make the files talk to each other. Also now that I know we can share methods within modules that's really helpful!
What is accomplished with raise ArgumentError? What do you think it's doing? Lets the user know that what they're trying to do isn't allowed by my code, and by default kicks the code out of the method. So, if you put that at the top of a method to check for some base requirement, if that requirement isn't fulfilled, it will kick you out and not run any more code - quicker, faster, less work.
Why do you think we made the .all & .find methods class methods? Why not instance methods? Because it's looking through a lot of accounts to find one or list all accounts, not creating or discussing the information regarding one specific account.
Why does it make sense to use inheritance for the different types of accounts? It makes the code dryer, less repetitive. You can assume it will act the same unless you tell it to act differently!
Did the presence of automated tests change the way you thought about the problem? How? Helped me slow down and write more specific code for each element required, instead of throwing a bunch of stuff at the wall and seeing what stuck :)

@kariabancroft
Copy link

Bank Account

What We're Looking For

Feature Feedback
Wave 1
All provided tests pass Yes. I liked the way you split out different functions into different methods. Like set_balance and argument to generalize the ArgumentErrors you were using.
Using the appropriate attr_ for instance variables Yes - using both attr_accessor and attr_reader for different properties.
Wave 2
All stubbed tests are implemented fully and pass Almost! The tests for the all look good - though I'd probably use a different variable name other than @new_bank since that doesn't say "list of accounts" to me. It looks like the find tests are calling the find method but there aren't any assertions.
Created instances (by calling new) in Account.all Yes
Used CSV library only in Account.all (not in Account.find) No - the find method is re-reading the CSV file rather than using the all method that is already reading the file. It is better to use the all method instead of re-reading the file because if the file changed or the way we wanted to access all of the accounts, we would have to change the find account then too.
Used Account.all to get account list in Account.find No - see above.
Wave 3
All stubbed tests are implemented fully and pass Yes. Nice job using the before block to DRY up your tests. Nice job using the times loop to DRY up the check tests.
Used inheritance in the initialize for both Checking and Savings Accounts (min balance) Yes - your set_balance method makes it nicely shared with the savings accounts. Sort of unrelated suggestion: it would be good to create a constant that referenced the minimum balance so if this changed you would be able to change it in only one place.
Used inheritance for the withdraw in checking and savings accounts Yes - you did a nice job moving the super call to the appropriate location to ensure you still had the behavior you were expecting.
Baseline
Used Git Regularly Yes - you did a really nice job with the frequency and commit messages
Answer comprehension questions Yes. I liked your responses on the class methods question, inheritance and TDD.
Extras Nice job with the MoneyMarket optional (especially writing the tests)!
Overall Nice job with the learning goals on this assignment!

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