HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Operations to a bank account

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
bankoperationsaccount

Problem

I am a new .NET/C# developer and one issue bugs me ever since I've wrote code the first time. How can I improve this code?

Program.cs:

```
class Program
{
static void Main(string[] args)
{
App();
}

public static void App()
{
BankDetails bankdet = new BankDetails();

ShowMenu();

while (true)
{
Console.WriteLine("");

string userInput = Console.ReadLine();
switch (userInput)
{
case "a":
Console.WriteLine("'Citire din fisier' selected");
break;
case "b":
Console.WriteLine("'Creare cont' selected");
Console.WriteLine("");
bankdet.CreateAccount("Name 1");
bankdet.CreateAccount("Name 2");
bankdet.CreateAccount("");
bankdet.CreateAccount("Name 4");
bankdet.CreateAccount("Name 5");
break;
case "c":
Console.WriteLine("'Depunere bancara' selected");
bankdet.Deposit();
break;
case "d":
Console.WriteLine("'Retragere bancara' selected");
bankdet.Withdraw();
break;
case "e":
Console.WriteLine("'Afisare sold' selected");
bankdet.Balance();
break;
case "f":
Environment.Exit(0);
break;
default:
Console.WriteLine("Please select a valid option");
break;
}
}
}

public static void ShowMenu()

Solution

Account Structure

Since it looks like you are trying to model the bank account slightly more realistically than a typical getting started tutorial, I'd like to address the conceptual structure of your BankAccount class.

In real life, an account is much more likely to be composed of a collection of transactions, rather than an updated balance.
Getting the balance would be accomplished by a method which sums up all the transactions.

So instead of (simplified):

class BankAccount //balance is data, transactions are methods.
{
    decimal balance;
    void Deposit(decimal amount) { balance += amount; }
    void Withdraw(decimal amount) { balance -= amount; }
}


You would have something like:

class Transaction
{
    decimal amount;
    //other details
}

class BankAccount //transactions are data, balance is method.
{
    List transactions;
    void AddTransaction(Transaction t) { transactions.Add(t); }
    decimal GetBalance() { /* return sum of all transactions */ }
}


Not only does this allow you to show your customer the details of the transactions made on their account, but it allows you to retroactively cancel an erroneous transaction and have the balance update automatically.

Floating Point

Another important issue is your choice of float as a representation of currency. This is inherently flawed, as floating point numbers are not meant to represent base-10 values with accuracy.

For this exact reason we have the System.Decimal type in c#.

Redundancy (one example)

if (ba != null)
{
   return ba;
}
else
{
   return null;
}


Can be replaced with

return ba;

Code Snippets

class BankAccount //balance is data, transactions are methods.
{
    decimal balance;
    void Deposit(decimal amount) { balance += amount; }
    void Withdraw(decimal amount) { balance -= amount; }
}
class Transaction
{
    decimal amount;
    //other details
}

class BankAccount //transactions are data, balance is method.
{
    List<Transaction> transactions;
    void AddTransaction(Transaction t) { transactions.Add(t); }
    decimal GetBalance() { /* return sum of all transactions */ }
}
if (ba != null)
{
   return ba;
}
else
{
   return null;
}

Context

StackExchange Code Review Q#106631, answer score: 14

Revisions (0)

No revisions yet.