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 - Janice Lichtman - Bank Account #27

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

Conversation

J-C-L
Copy link

@J-C-L J-C-L commented Feb 27, 2017

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? By using modules, related classes are grouped together. This allows them to be named in simple language that makes sense in the given context, without concern about the same words (like "account") being used for classes in another context.
What is accomplished with raise ArgumentError? What do you think it's doing? When an error is raised, the program aborts and outputs a message. This is useful when a case (or state) arises that the program is not built to handle.
Why do you think we made the .all & .find methods class methods? Why not instance methods? For .all and .find we need to consider all the instances of a class, so we are looking at the level of the whole class. It would not make sense to only look at a single instance to gain information about the 'whole group'.
Why does it make sense to use inheritance for the different types of accounts? This makes sense because although each account type has some unique features, they all share some basic parameters and functionality. As said in the name, each "account type" IS-AN "account", so it makes sense for the account types to be subclasses of Account and inherit the properties of "accounts".

|
| Did the presence of automated tests change the way you thought about the problem? How? |The automated tests forced me to focus on the specific behaviors I needed to code, and to make sure they were functioning in the desired manner. Writing the tests first, helped me clarify exactly what I needed from each piece of functionality I was writing. |

@PilgrimMemoirs
Copy link

PilgrimMemoirs commented Feb 27, 2017

Bank Account

What We're Looking For

Feature Feedback
Wave 1
All provided tests pass Well Done
Using the appropriate attr_ for instance variables Well Done
Wave 2
All stubbed tests are implemented fully and pass Well Done
Created instances (by calling new) in Account.all .read_csv file code could directly be in all instead, or at least called in it to return all the account objects. On the right idea, so it looks good.
Used CSV library only in Account.all (not in Account.find) Instead of using a class variable, could have used Account.all to create and return a collection of instances of Account. Then the method could be called anytime you need to perform something on all accounts, like in find.
Used Account.all to get account list in Account.find n/a - used a class variable instead
Wave 3
Used inheritance in the initialize for both Checking and Savings Accounts (min balance) Well Done
Used inheritance for the withdraw in checking and savings accounts Withdraw methods were completely rewritten. How could you still use the functionality of the super class' withdraw method in the child classes? Hint: super can still be used in a method with the same name. And you can still add additional functionality!
Tests By setting up the Account.all method to return a collection of Account classes, you won't need to call .reset_all_accounts_for_test or .read_csv multiple times in your tests. "gives correct values for the ID and balance of the first and last accounts match what's in the CSV file" test should not need to reset the accounts every time.

|
| Baseline | |
| Used Git Regularly | committed pretty often. Instead of committing with completion of a wave, commit when a new feature, like a test/method, is working. Then give a specific commit message to what thing was added or changed. |
| Answer comprehension questions | Well Done |
| Extras | Nice addition of Optionals! Very extensive testing, nice! |

Summary:

  • Avoid using class variables, instead of .all return collection.
  • Utilize inheritance by having methods, like withdraw, use the functionality of it's inherited class' method with customization.
  • Identify code that is being called a lot, how can it be more efficient

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