patterncsharpModerate
Operations to a bank account
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()
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
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):
You would have something like:
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
For this exact reason we have the System.Decimal type in c#.
Redundancy (one example)
Can be replaced with
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.