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

Queues - BankAccounts - Maria McGrew #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Queues - BankAccounts - Maria McGrew #49

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 9, 2017

Bank Account

Congratulations! You're submitting your assignment.

Comprehension Questions

Question Answer Instructor Feedback
Why is it useful to put classes inside modules? What do you think? I think it is useful since the class gains all the functionality of the methods within the module. So, like what I noticed happened with the checking accounts, some testing didn't need to be redone since it was referenced in other modules. The big reason to use modules is to provide a namespace for your program. This helps to group related pieces together (Account, CheckingAccount and SavingsAccount are all grouped under Bank) and to make naming things easier (maybe a teller at the bank has an Account used to log into the computer systems - we don't want this to collide with our money accounts).
What is accomplished with raise ArgumentError? What do you think it's doing? The method checks for the statement in the argument, such as "must be an array" and if the argument meets that criteria, then it runs through the rest of the method. If the criteria isn't met, then the argument error is returned and the method stops and returns the error message. This answer is a little confusing to me. Make sure you understand the difference between raise-ing an exception and using must_something in test code.
Why do you think we made the .all & .find methods class methods? Why not instance methods? So that the methods we built within the class, the instance methods, could be used on various instances of the class methods, like when Bank::Accounts was used in other files. This talks about what instance methods are for, but it doesn't answer the question about why .all and .find are class methods
Why does it make sense to use inheritance for the different types of accounts? So that the methods could be shared without having to rewrite/redefine them for each class and so that if a method needs to be changed, it only needs to be changed in one place rather than in all classes. This is seen when super was used between the accounts since all accounts had the some functionality when it came to account ID, balance, and open date. Good response
Did the presence of automated tests change the way you thought about the problem? How? Yes, for me it is very different since it seems as though we're approaching possible scenarios with the tests, and then responding in the code by later defining the methods that were already used in the tests. At first it seemed counterintuitive to me, but as I watched the video and worked through the Scrabble project, it is becoming clearer how to phrase the tests and what test methods to use in order to develop the code. Good response

@droberts-sea
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
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
All stubbed tests are implemented fully and pass yes
Used inheritance in the initialize for both Checking and Savings Accounts (min balance) yes
Used inheritance for the withdraw in checking and savings accounts yes
Baseline
Used Git Regularly In the future, please try to break your work up into many small commits - at least one per method you implement. That gives us a better sense of how you're progressing, and will be a good habit to have once you get out into industry.
Answer comprehension questions see inline responses in your PR comment

Outside of improving your git habits, I'm quite happy with this code, particularly the way inheritance is used. Great work!

@@ -18,17 +19,12 @@
end

it "Raises an ArgumentError when created with a negative balance" do
# Note: we haven't talked about procs yet. You can think

Choose a reason for hiding this comment

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

Why did you remove all these comments?

@account.withdraw_using_check(10)
@account.withdraw_using_check(10)
@account.withdraw_using_check(10)
@account.withdraw_using_check(10)

Choose a reason for hiding this comment

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

Could you use something like a times loop here to dry up this 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.

1 participant