aksnell / assignments

0 stars 0 forks source link

02 - 03 - First Bank of Suncoast - #7

Closed aksnell closed 4 years ago

aksnell commented 4 years ago

For this assignment, you will be creating your own personal bank account manager. You will be creating an app that will let you track both a savings account and a checking account total by performing transactions, such as withdrawals, deposits, and transfers. The application will also save your information in a file so you can track your account totals over time.

Objectives

Requirements

Create a console app that allows a user to manage a banking account.

As user should be able to make transactions against their accounts.

The transaction information should be stored in a file, using a CSV(or JSON) format to record the data.

Balances will be computed by determining the cumulative effect of all the transactions in order. For instance, if a user deposits 10 to their savings account and then withdraws 8 from their savings account, their balance is computed as 2.

Explorer Mode

Adventure Mode

Epic Mode

Additional Resources

aksnell commented 4 years ago

https://github.com/aksnell/suncoast-bank

gstark commented 4 years ago

Do you need this default constructor? https://github.com/aksnell/suncoast-bank/blob/master/Account.cs#L13

If you don't need to access these from outside remove the get: https://github.com/aksnell/suncoast-bank/blob/master/Account.cs#L10-L11 -- if you don't set them from outside you can remove the get and make them normal internal private properties.

https://github.com/aksnell/suncoast-bank/blob/master/Account.cs#L30 -- Never invent your own cryptography -- and use a recognized strong algorithm when you can. I suggest checking out https://github.com/BcryptNet/bcrypt.net

You can also make a "custom set" function where you can have public string Password and have a set method that really just assigns to the passwordhash and encrypted password. In essence making a virtual property. One that isn't backed by a value but is instead a facade for setting the hashed password and salt.

Does a Bank really have one account ID? This really seems more like Account where your Account seems more like a User or Person or Customer -- names matter.

Ugh, magic numbers: https://github.com/aksnell/suncoast-bank/blob/master/Constants.cs#L5-L22 -- avoid these whenever possible. If you must, I would suggest using an enum instead or just descriptive strings. See enums: https://docs.microsoft.com/en-us/dotnet/csharp/language-reference/builtin-types/enum -- By using an enum you are passing around a specific type instead of masquarading things as int

Also use names from the problem domain. Banks don't call int UNDERFLOW and OVERFLOW, those are programming terms. Always code in the language of the problem. So OVERDRAFT and UNDERDRAFT. or OVERDRAFT_ATTEMPT and UNDERDRAFT_ATTEMPT.

https://github.com/aksnell/suncoast-bank/blob/master/Bank.cs#L148 00 https://github.com/aksnell/suncoast-bank/blob/master/Bank.cs#L158-L159

Here is a case where perhaps an inherited class does help. Rather than have to pass a transaction type perhaps there should be a WithdrawTransaction and a DepositTransaction and they can be managed more intelligently that way. Displaying, totalling, etc.

This line of code: https://github.com/aksnell/suncoast-bank/blob/master/Bank.cs#L168 cannot stand without explaining why you are doing fancy math. This is also a symptom of using magic numbers. Magic numbers beget magic implementation in your code. And this magic cannot stand without explaining. Again, you are favoring fancy side effects.

https://github.com/aksnell/suncoast-bank/blob/master/Bank.cs#L192 - you don't need this else. When the if true part ends in a return there is no way to access the remaining code so you are free to not have an else and let that code flow freely.

Here is a perfect example where, if you had used subclasses you could have avoided a case statement. It even fits nicely the pattern of our child class "A WithdrawTransaction is a Transaction, a DepositTransaction is a transaction" - Try implementing the two types of transactions as inherited from a generic base class and see how that influences the design https://github.com/aksnell/suncoast-bank/blob/master/Bank.cs#L228

This is a comment that adds nothing to the context of the code. https://github.com/aksnell/suncoast-bank/blob/master/FrontEnd.cs#L36 Either improve the comment or get rid of it.

Here is another hint that this Account class is really a User -- userAccount as a variable name: https://github.com/aksnell/suncoast-bank/blob/master/Portal.cs#L35

gstark commented 4 years ago

This code style here is improved from the previous code. There is still some "fancy for fancy sake", e.g. the ^ 1 code. So I commend you for working on simplifying things. Try some of the above suggestions.

aksnell commented 4 years ago

Your homework 02 - 03 - First Bank of Suncoast - was marked: Exceeds Expectations

Woooooh!

“Woooooh!” — via Mark Dewey

gstark commented 4 years ago

Also be more careful with the spelling and meaning of commits. This commit message doesn't match what the commit actually does: https://github.com/aksnell/suncoast-bank/commit/a9542bf74571cd4e10b54ac4858c8d604b3c8209

gstark commented 4 years ago

So it does look like CsvReader uses C#'s built-in introspection to find the appropriate constructor. So if you name the constructor arguments after the matching column (and property) names then it will find and use it.

Here is some sample code I wrote to see how the introspection works:

using System;

namespace SuncoastBank
{
    class Program
    {
        static void IntrospectConstuctors()
        {
            Console.WriteLine("---- Introspect constructors ----");
            var constructors = typeof(Transaction).GetConstructors();

            foreach (var constructor in constructors)
            {
                Console.WriteLine(constructor.Name);
                foreach (var parameter in constructor.GetParameters())
                {
                    var type = parameter.GetType();
                    var name = parameter.Name;

                    Console.WriteLine($"    Parameter named {name} is type {type}");
                }
            }
        }

        static void Main(string[] args)
        {
            IntrospectConstuctors();

            var portal = new Portal();
            portal.Login();
        }
    }
}