Simulation of a banking system with an Account class in C++ Announcing the arrival of Valued Associate #679: Cesar Manara Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)Simple container class with templatesOOP bank databaseUsing PostgreSQL effectively with a banking appAccount registration system for a quiz gameFake bank account classBanking system programRuby banking system programCinema SimulationAn account/banking system in C++C++ Banking Class
8 Prisoners wearing hats
What are the out-of-universe reasons for the references to Toby Maguire-era Spider-Man in ITSV
Do wooden building fires get hotter than 600°C?
Extracting terms with certain heads in a function
Did MS DOS itself ever use blinking text?
How to react to hostile behavior from a senior developer?
What font is "z" in "z-score"?
Can anything be seen from the center of the Boötes void? How dark would it be?
Fundamental Solution of the Pell Equation
An adverb for when you're not exaggerating
What do you call a floor made of glass so you can see through the floor?
Maximum summed powersets with non-adjacent items
Can melee weapons be used to deliver Contact Poisons?
Using audio cues to encourage good posture
How to Make a Beautiful Stacked 3D Plot
Trademark violation for app?
How do I make this wiring inside cabinet safer? (Pic)
How can I use the Python library networkx from Mathematica?
What is homebrew?
Can you use the Shield Master feat to shove someone before you make an attack by using a Readied action?
How come Sam didn't become Lord of Horn Hill?
If my PI received research grants from a company to be able to pay my postdoc salary, did I have a potential conflict interest too?
How could we fake a moon landing now?
How do I stop a creek from eroding my steep embankment?
Simulation of a banking system with an Account class in C++
Announcing the arrival of Valued Associate #679: Cesar Manara
Planned maintenance scheduled April 17/18, 2019 at 00:00UTC (8:00pm US/Eastern)Simple container class with templatesOOP bank databaseUsing PostgreSQL effectively with a banking appAccount registration system for a quiz gameFake bank account classBanking system programRuby banking system programCinema SimulationAn account/banking system in C++C++ Banking Class
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty,.everyoneloves__bot-mid-leaderboard:empty margin-bottom:0;
$begingroup$
I have been learning C++ and recently I have started practicing classes. I made this simulation of a banking system with an Account class. I would like to hear feedback on my code, what is good - what I should keep doing, and especially what I should pay more attention to, or if you have any suggestion on how to simplify parts of the code, make it more readable because I feel this could be improved. Also, I would like to hear comments on exception handling, since I am quite new to that as well. Thank you all in advance, I will do my best to implement any of your suggestions.
Account.h
#ifndef ACCOUNT.H
#define ACCOUNT.H
#include <string>
#include <vector>
class Account
std::string name;
int id;
double balance;
public:
Account();
std::string getName() const;
int getId() const;
double getBalance() const;
void setName(std::string);
void setID(int);
void setBalance(double);
void addAccount (Account);
void withdraw(double);
void deposit(double);
static std::vector<Account> accountDatabase;
;
#endif // ACCOUNT
Account.cpp
#include "Account.h"
#include <iostream>
#include <string>
#include <vector>
Account::Account()
name = "";
id = 0;
balance = 0;
std::vector<Account> Account::accountDatabase;
void Account::addAccount(Account account)
accountDatabase.push_back(account);
std::string Account::getName() const
return name;
int Account::getId() const
return id;
double Account::getBalance() const
return balance;
void Account::setName(std::string userName)
name = userName;
void Account::setID(int newId)
if (newId < 1)
throw "ntttt ~ ID cannot be zero or negative ~";
for (int i = 0; i < accountDatabase.size(); i++)
if (newId == accountDatabase[i].getId())
throw "ntttt~ Entered ID is already in use ~";
id = newId;
void Account::setBalance(double newBalance)
if (newBalance < 0)
throw "ntttt ~ Balance cannot be negative ~";
balance = newBalance;
void Account::withdraw(double amount)
if (amount < 0)
throw "ntttt ~ Withdrawal amount cannot be negative ~";
balance -= amount;
void Account::deposit(double amount)
if (amount < 0)
throw "ntttt ~ Amount for deposit cannot be negative ~";
balance += amount;
Main
#include <iostream>
#include "Account.h"
#include <string>
void printMenu()
std::cout << "n" << R"(
Please select one of the following options:
1. Create an account
2. Check balance
3. Withdraw
4. Deposit
5. Account summary
6. Make a transaction
7. Exit
)" << "ntttt--> ";
// get a valid input
template<typename Type>
void getInput(Type &value)
while (true)
std::cin >> value;
if (std::cin.fail())
std::cin.clear();
std::cin.ignore(100, 'n');
std::cout << "nttttt~ Invalid input ~"
<< "ntttt--> Enter again: ";
else
std::cin.ignore();
return;
// find account in account database and return index of that account
int findAccount (int id)
for (int i = 0; i < Account::accountDatabase.size(); i++)
if (id == Account::accountDatabase[i].getId()) return i;
return -1;
void createAccount ()
Account newAccount;
std::cout << "ntttt--> Please enter your name: ";
std::string name;
std::cin.ignore();
std::getline(std::cin, name);
newAccount.setName(name);
std::cout << "ntttt--> Please enter your ID: ";
int id;
getInput(id);
newAccount.setID(id);
std::cout << "ntttt--> Please enter your balance: ";
double balance;
getInput(balance);
newAccount.setBalance(balance);
// add account to the database
newAccount.addAccount(newAccount);
std::cout << "ntttt~ Your account has been successfully created ~n";
void MenuSelection()
int option = 1, account, id;
while (option != 7)
std::cout << R"(
########################################################################################################################
~ THANK YOU FOR USING OUR SERVICES ~
########################################################################################################################
)";
int main()
std::cout << R"(
########################################################################################################################
~ W E L C O M E T O O U R B A N K ~
########################################################################################################################
)";
MenuSelection();
return 0;
c++ beginner object-oriented error-handling vectors
$endgroup$
add a comment |
$begingroup$
I have been learning C++ and recently I have started practicing classes. I made this simulation of a banking system with an Account class. I would like to hear feedback on my code, what is good - what I should keep doing, and especially what I should pay more attention to, or if you have any suggestion on how to simplify parts of the code, make it more readable because I feel this could be improved. Also, I would like to hear comments on exception handling, since I am quite new to that as well. Thank you all in advance, I will do my best to implement any of your suggestions.
Account.h
#ifndef ACCOUNT.H
#define ACCOUNT.H
#include <string>
#include <vector>
class Account
std::string name;
int id;
double balance;
public:
Account();
std::string getName() const;
int getId() const;
double getBalance() const;
void setName(std::string);
void setID(int);
void setBalance(double);
void addAccount (Account);
void withdraw(double);
void deposit(double);
static std::vector<Account> accountDatabase;
;
#endif // ACCOUNT
Account.cpp
#include "Account.h"
#include <iostream>
#include <string>
#include <vector>
Account::Account()
name = "";
id = 0;
balance = 0;
std::vector<Account> Account::accountDatabase;
void Account::addAccount(Account account)
accountDatabase.push_back(account);
std::string Account::getName() const
return name;
int Account::getId() const
return id;
double Account::getBalance() const
return balance;
void Account::setName(std::string userName)
name = userName;
void Account::setID(int newId)
if (newId < 1)
throw "ntttt ~ ID cannot be zero or negative ~";
for (int i = 0; i < accountDatabase.size(); i++)
if (newId == accountDatabase[i].getId())
throw "ntttt~ Entered ID is already in use ~";
id = newId;
void Account::setBalance(double newBalance)
if (newBalance < 0)
throw "ntttt ~ Balance cannot be negative ~";
balance = newBalance;
void Account::withdraw(double amount)
if (amount < 0)
throw "ntttt ~ Withdrawal amount cannot be negative ~";
balance -= amount;
void Account::deposit(double amount)
if (amount < 0)
throw "ntttt ~ Amount for deposit cannot be negative ~";
balance += amount;
Main
#include <iostream>
#include "Account.h"
#include <string>
void printMenu()
std::cout << "n" << R"(
Please select one of the following options:
1. Create an account
2. Check balance
3. Withdraw
4. Deposit
5. Account summary
6. Make a transaction
7. Exit
)" << "ntttt--> ";
// get a valid input
template<typename Type>
void getInput(Type &value)
while (true)
std::cin >> value;
if (std::cin.fail())
std::cin.clear();
std::cin.ignore(100, 'n');
std::cout << "nttttt~ Invalid input ~"
<< "ntttt--> Enter again: ";
else
std::cin.ignore();
return;
// find account in account database and return index of that account
int findAccount (int id)
for (int i = 0; i < Account::accountDatabase.size(); i++)
if (id == Account::accountDatabase[i].getId()) return i;
return -1;
void createAccount ()
Account newAccount;
std::cout << "ntttt--> Please enter your name: ";
std::string name;
std::cin.ignore();
std::getline(std::cin, name);
newAccount.setName(name);
std::cout << "ntttt--> Please enter your ID: ";
int id;
getInput(id);
newAccount.setID(id);
std::cout << "ntttt--> Please enter your balance: ";
double balance;
getInput(balance);
newAccount.setBalance(balance);
// add account to the database
newAccount.addAccount(newAccount);
std::cout << "ntttt~ Your account has been successfully created ~n";
void MenuSelection()
int option = 1, account, id;
while (option != 7)
std::cout << R"(
########################################################################################################################
~ THANK YOU FOR USING OUR SERVICES ~
########################################################################################################################
)";
int main()
std::cout << R"(
########################################################################################################################
~ W E L C O M E T O O U R B A N K ~
########################################################################################################################
)";
MenuSelection();
return 0;
c++ beginner object-oriented error-handling vectors
$endgroup$
add a comment |
$begingroup$
I have been learning C++ and recently I have started practicing classes. I made this simulation of a banking system with an Account class. I would like to hear feedback on my code, what is good - what I should keep doing, and especially what I should pay more attention to, or if you have any suggestion on how to simplify parts of the code, make it more readable because I feel this could be improved. Also, I would like to hear comments on exception handling, since I am quite new to that as well. Thank you all in advance, I will do my best to implement any of your suggestions.
Account.h
#ifndef ACCOUNT.H
#define ACCOUNT.H
#include <string>
#include <vector>
class Account
std::string name;
int id;
double balance;
public:
Account();
std::string getName() const;
int getId() const;
double getBalance() const;
void setName(std::string);
void setID(int);
void setBalance(double);
void addAccount (Account);
void withdraw(double);
void deposit(double);
static std::vector<Account> accountDatabase;
;
#endif // ACCOUNT
Account.cpp
#include "Account.h"
#include <iostream>
#include <string>
#include <vector>
Account::Account()
name = "";
id = 0;
balance = 0;
std::vector<Account> Account::accountDatabase;
void Account::addAccount(Account account)
accountDatabase.push_back(account);
std::string Account::getName() const
return name;
int Account::getId() const
return id;
double Account::getBalance() const
return balance;
void Account::setName(std::string userName)
name = userName;
void Account::setID(int newId)
if (newId < 1)
throw "ntttt ~ ID cannot be zero or negative ~";
for (int i = 0; i < accountDatabase.size(); i++)
if (newId == accountDatabase[i].getId())
throw "ntttt~ Entered ID is already in use ~";
id = newId;
void Account::setBalance(double newBalance)
if (newBalance < 0)
throw "ntttt ~ Balance cannot be negative ~";
balance = newBalance;
void Account::withdraw(double amount)
if (amount < 0)
throw "ntttt ~ Withdrawal amount cannot be negative ~";
balance -= amount;
void Account::deposit(double amount)
if (amount < 0)
throw "ntttt ~ Amount for deposit cannot be negative ~";
balance += amount;
Main
#include <iostream>
#include "Account.h"
#include <string>
void printMenu()
std::cout << "n" << R"(
Please select one of the following options:
1. Create an account
2. Check balance
3. Withdraw
4. Deposit
5. Account summary
6. Make a transaction
7. Exit
)" << "ntttt--> ";
// get a valid input
template<typename Type>
void getInput(Type &value)
while (true)
std::cin >> value;
if (std::cin.fail())
std::cin.clear();
std::cin.ignore(100, 'n');
std::cout << "nttttt~ Invalid input ~"
<< "ntttt--> Enter again: ";
else
std::cin.ignore();
return;
// find account in account database and return index of that account
int findAccount (int id)
for (int i = 0; i < Account::accountDatabase.size(); i++)
if (id == Account::accountDatabase[i].getId()) return i;
return -1;
void createAccount ()
Account newAccount;
std::cout << "ntttt--> Please enter your name: ";
std::string name;
std::cin.ignore();
std::getline(std::cin, name);
newAccount.setName(name);
std::cout << "ntttt--> Please enter your ID: ";
int id;
getInput(id);
newAccount.setID(id);
std::cout << "ntttt--> Please enter your balance: ";
double balance;
getInput(balance);
newAccount.setBalance(balance);
// add account to the database
newAccount.addAccount(newAccount);
std::cout << "ntttt~ Your account has been successfully created ~n";
void MenuSelection()
int option = 1, account, id;
while (option != 7)
std::cout << R"(
########################################################################################################################
~ THANK YOU FOR USING OUR SERVICES ~
########################################################################################################################
)";
int main()
std::cout << R"(
########################################################################################################################
~ W E L C O M E T O O U R B A N K ~
########################################################################################################################
)";
MenuSelection();
return 0;
c++ beginner object-oriented error-handling vectors
$endgroup$
I have been learning C++ and recently I have started practicing classes. I made this simulation of a banking system with an Account class. I would like to hear feedback on my code, what is good - what I should keep doing, and especially what I should pay more attention to, or if you have any suggestion on how to simplify parts of the code, make it more readable because I feel this could be improved. Also, I would like to hear comments on exception handling, since I am quite new to that as well. Thank you all in advance, I will do my best to implement any of your suggestions.
Account.h
#ifndef ACCOUNT.H
#define ACCOUNT.H
#include <string>
#include <vector>
class Account
std::string name;
int id;
double balance;
public:
Account();
std::string getName() const;
int getId() const;
double getBalance() const;
void setName(std::string);
void setID(int);
void setBalance(double);
void addAccount (Account);
void withdraw(double);
void deposit(double);
static std::vector<Account> accountDatabase;
;
#endif // ACCOUNT
Account.cpp
#include "Account.h"
#include <iostream>
#include <string>
#include <vector>
Account::Account()
name = "";
id = 0;
balance = 0;
std::vector<Account> Account::accountDatabase;
void Account::addAccount(Account account)
accountDatabase.push_back(account);
std::string Account::getName() const
return name;
int Account::getId() const
return id;
double Account::getBalance() const
return balance;
void Account::setName(std::string userName)
name = userName;
void Account::setID(int newId)
if (newId < 1)
throw "ntttt ~ ID cannot be zero or negative ~";
for (int i = 0; i < accountDatabase.size(); i++)
if (newId == accountDatabase[i].getId())
throw "ntttt~ Entered ID is already in use ~";
id = newId;
void Account::setBalance(double newBalance)
if (newBalance < 0)
throw "ntttt ~ Balance cannot be negative ~";
balance = newBalance;
void Account::withdraw(double amount)
if (amount < 0)
throw "ntttt ~ Withdrawal amount cannot be negative ~";
balance -= amount;
void Account::deposit(double amount)
if (amount < 0)
throw "ntttt ~ Amount for deposit cannot be negative ~";
balance += amount;
Main
#include <iostream>
#include "Account.h"
#include <string>
void printMenu()
std::cout << "n" << R"(
Please select one of the following options:
1. Create an account
2. Check balance
3. Withdraw
4. Deposit
5. Account summary
6. Make a transaction
7. Exit
)" << "ntttt--> ";
// get a valid input
template<typename Type>
void getInput(Type &value)
while (true)
std::cin >> value;
if (std::cin.fail())
std::cin.clear();
std::cin.ignore(100, 'n');
std::cout << "nttttt~ Invalid input ~"
<< "ntttt--> Enter again: ";
else
std::cin.ignore();
return;
// find account in account database and return index of that account
int findAccount (int id)
for (int i = 0; i < Account::accountDatabase.size(); i++)
if (id == Account::accountDatabase[i].getId()) return i;
return -1;
void createAccount ()
Account newAccount;
std::cout << "ntttt--> Please enter your name: ";
std::string name;
std::cin.ignore();
std::getline(std::cin, name);
newAccount.setName(name);
std::cout << "ntttt--> Please enter your ID: ";
int id;
getInput(id);
newAccount.setID(id);
std::cout << "ntttt--> Please enter your balance: ";
double balance;
getInput(balance);
newAccount.setBalance(balance);
// add account to the database
newAccount.addAccount(newAccount);
std::cout << "ntttt~ Your account has been successfully created ~n";
void MenuSelection()
int option = 1, account, id;
while (option != 7)
std::cout << R"(
########################################################################################################################
~ THANK YOU FOR USING OUR SERVICES ~
########################################################################################################################
)";
int main()
std::cout << R"(
########################################################################################################################
~ W E L C O M E T O O U R B A N K ~
########################################################################################################################
)";
MenuSelection();
return 0;
c++ beginner object-oriented error-handling vectors
c++ beginner object-oriented error-handling vectors
asked Apr 13 at 22:00
J.JJ.J
414
414
add a comment |
add a comment |
2 Answers
2
active
oldest
votes
$begingroup$
I have a few suggestions:
ntttt
is everywhere, move it into a function which takes and returns a string, and prepends whatever string it is given with this format - the benefit of this is that if you change your formatting in the future, you only need to do it in one place.- Getting the user to enter their ID is a common operation, so put this logic all in one place. i.e.
std::cout << "ntttt--> Please enter your ID: ";
getInput(id);
account = findAccount(id);
Should be lopped out into its own function, again for the same reason - if the way you want to do it changes, you only need to do it in one place! - Move the code within each case block into its own function, e.g for
case 2
, just make a function calledcheckBalance
which does exactly that. Call it from the case block (similar to how you have done forcase 1
). The comments already hint at what each block does, but the whole of the switch/case statement is quite a lot of code! - The way you look up accounts is a little confusing and potentially inefficient for large numbers of accounts, I would use a map (http://www.cplusplus.com/reference/map/map/) with the key being the ID.
I hope this is enough to get you on your way, if you give this another stab then I'm happy to take another look!
$endgroup$
add a comment |
$begingroup$
The class is doing too much: it is not the task of the
Account
class to also keep a database of all accounts. Remember: one class (or function), one responsibility. By the same principle,menuSelection
does too much in your main program.Use the initializer list in your constructor, i.e., do
Account::Account() : name_(), id_(0), balance(0)
. But even better, you should always try to avoid defining default operations, as per C.20 of the C++ Core Guidelines.Pass complex (i.e., not built-in types) by const-ref. This includes strings and Account types. For example, rather do
void Account::setName(const std::string& userName) ...
. Another possibility - if your compiler is recent enough - is to use std::string_view.Avoid throwing character strings and throw proper objects instead, see E.14.
You seem to have an invariant which says that the balance can never be negative. I would enforce this more consistently by writing a private function like
void checkInvariant() ...
that makes things likeassert(balance > 0)
(but be careful when comparing floating point values to constant values; it should be done with a threshold). Then, you can add this check to suitable functions and the invariant is nicely enforced by the class itself.The above can help you catch errors like in
withdraw
: is it acceptable that the amount becomes negative here?
$endgroup$
add a comment |
Your Answer
StackExchange.ifUsing("editor", function ()
StackExchange.using("externalEditor", function ()
StackExchange.using("snippets", function ()
StackExchange.snippets.init();
);
);
, "code-snippets");
StackExchange.ready(function()
var channelOptions =
tags: "".split(" "),
id: "196"
;
initTagRenderer("".split(" "), "".split(" "), channelOptions);
StackExchange.using("externalEditor", function()
// Have to fire editor after snippets, if snippets enabled
if (StackExchange.settings.snippets.snippetsEnabled)
StackExchange.using("snippets", function()
createEditor();
);
else
createEditor();
);
function createEditor()
StackExchange.prepareEditor(
heartbeatType: 'answer',
autoActivateHeartbeat: false,
convertImagesToLinks: false,
noModals: true,
showLowRepImageUploadWarning: true,
reputationToPostImages: null,
bindNavPrevention: true,
postfix: "",
imageUploader:
brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
allowUrls: true
,
onDemand: true,
discardSelector: ".discard-answer"
,immediatelyShowMarkdownHelp:true
);
);
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f217415%2fsimulation-of-a-banking-system-with-an-account-class-in-c%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
$begingroup$
I have a few suggestions:
ntttt
is everywhere, move it into a function which takes and returns a string, and prepends whatever string it is given with this format - the benefit of this is that if you change your formatting in the future, you only need to do it in one place.- Getting the user to enter their ID is a common operation, so put this logic all in one place. i.e.
std::cout << "ntttt--> Please enter your ID: ";
getInput(id);
account = findAccount(id);
Should be lopped out into its own function, again for the same reason - if the way you want to do it changes, you only need to do it in one place! - Move the code within each case block into its own function, e.g for
case 2
, just make a function calledcheckBalance
which does exactly that. Call it from the case block (similar to how you have done forcase 1
). The comments already hint at what each block does, but the whole of the switch/case statement is quite a lot of code! - The way you look up accounts is a little confusing and potentially inefficient for large numbers of accounts, I would use a map (http://www.cplusplus.com/reference/map/map/) with the key being the ID.
I hope this is enough to get you on your way, if you give this another stab then I'm happy to take another look!
$endgroup$
add a comment |
$begingroup$
I have a few suggestions:
ntttt
is everywhere, move it into a function which takes and returns a string, and prepends whatever string it is given with this format - the benefit of this is that if you change your formatting in the future, you only need to do it in one place.- Getting the user to enter their ID is a common operation, so put this logic all in one place. i.e.
std::cout << "ntttt--> Please enter your ID: ";
getInput(id);
account = findAccount(id);
Should be lopped out into its own function, again for the same reason - if the way you want to do it changes, you only need to do it in one place! - Move the code within each case block into its own function, e.g for
case 2
, just make a function calledcheckBalance
which does exactly that. Call it from the case block (similar to how you have done forcase 1
). The comments already hint at what each block does, but the whole of the switch/case statement is quite a lot of code! - The way you look up accounts is a little confusing and potentially inefficient for large numbers of accounts, I would use a map (http://www.cplusplus.com/reference/map/map/) with the key being the ID.
I hope this is enough to get you on your way, if you give this another stab then I'm happy to take another look!
$endgroup$
add a comment |
$begingroup$
I have a few suggestions:
ntttt
is everywhere, move it into a function which takes and returns a string, and prepends whatever string it is given with this format - the benefit of this is that if you change your formatting in the future, you only need to do it in one place.- Getting the user to enter their ID is a common operation, so put this logic all in one place. i.e.
std::cout << "ntttt--> Please enter your ID: ";
getInput(id);
account = findAccount(id);
Should be lopped out into its own function, again for the same reason - if the way you want to do it changes, you only need to do it in one place! - Move the code within each case block into its own function, e.g for
case 2
, just make a function calledcheckBalance
which does exactly that. Call it from the case block (similar to how you have done forcase 1
). The comments already hint at what each block does, but the whole of the switch/case statement is quite a lot of code! - The way you look up accounts is a little confusing and potentially inefficient for large numbers of accounts, I would use a map (http://www.cplusplus.com/reference/map/map/) with the key being the ID.
I hope this is enough to get you on your way, if you give this another stab then I'm happy to take another look!
$endgroup$
I have a few suggestions:
ntttt
is everywhere, move it into a function which takes and returns a string, and prepends whatever string it is given with this format - the benefit of this is that if you change your formatting in the future, you only need to do it in one place.- Getting the user to enter their ID is a common operation, so put this logic all in one place. i.e.
std::cout << "ntttt--> Please enter your ID: ";
getInput(id);
account = findAccount(id);
Should be lopped out into its own function, again for the same reason - if the way you want to do it changes, you only need to do it in one place! - Move the code within each case block into its own function, e.g for
case 2
, just make a function calledcheckBalance
which does exactly that. Call it from the case block (similar to how you have done forcase 1
). The comments already hint at what each block does, but the whole of the switch/case statement is quite a lot of code! - The way you look up accounts is a little confusing and potentially inefficient for large numbers of accounts, I would use a map (http://www.cplusplus.com/reference/map/map/) with the key being the ID.
I hope this is enough to get you on your way, if you give this another stab then I'm happy to take another look!
answered Apr 13 at 22:36
princeprince
22913
22913
add a comment |
add a comment |
$begingroup$
The class is doing too much: it is not the task of the
Account
class to also keep a database of all accounts. Remember: one class (or function), one responsibility. By the same principle,menuSelection
does too much in your main program.Use the initializer list in your constructor, i.e., do
Account::Account() : name_(), id_(0), balance(0)
. But even better, you should always try to avoid defining default operations, as per C.20 of the C++ Core Guidelines.Pass complex (i.e., not built-in types) by const-ref. This includes strings and Account types. For example, rather do
void Account::setName(const std::string& userName) ...
. Another possibility - if your compiler is recent enough - is to use std::string_view.Avoid throwing character strings and throw proper objects instead, see E.14.
You seem to have an invariant which says that the balance can never be negative. I would enforce this more consistently by writing a private function like
void checkInvariant() ...
that makes things likeassert(balance > 0)
(but be careful when comparing floating point values to constant values; it should be done with a threshold). Then, you can add this check to suitable functions and the invariant is nicely enforced by the class itself.The above can help you catch errors like in
withdraw
: is it acceptable that the amount becomes negative here?
$endgroup$
add a comment |
$begingroup$
The class is doing too much: it is not the task of the
Account
class to also keep a database of all accounts. Remember: one class (or function), one responsibility. By the same principle,menuSelection
does too much in your main program.Use the initializer list in your constructor, i.e., do
Account::Account() : name_(), id_(0), balance(0)
. But even better, you should always try to avoid defining default operations, as per C.20 of the C++ Core Guidelines.Pass complex (i.e., not built-in types) by const-ref. This includes strings and Account types. For example, rather do
void Account::setName(const std::string& userName) ...
. Another possibility - if your compiler is recent enough - is to use std::string_view.Avoid throwing character strings and throw proper objects instead, see E.14.
You seem to have an invariant which says that the balance can never be negative. I would enforce this more consistently by writing a private function like
void checkInvariant() ...
that makes things likeassert(balance > 0)
(but be careful when comparing floating point values to constant values; it should be done with a threshold). Then, you can add this check to suitable functions and the invariant is nicely enforced by the class itself.The above can help you catch errors like in
withdraw
: is it acceptable that the amount becomes negative here?
$endgroup$
add a comment |
$begingroup$
The class is doing too much: it is not the task of the
Account
class to also keep a database of all accounts. Remember: one class (or function), one responsibility. By the same principle,menuSelection
does too much in your main program.Use the initializer list in your constructor, i.e., do
Account::Account() : name_(), id_(0), balance(0)
. But even better, you should always try to avoid defining default operations, as per C.20 of the C++ Core Guidelines.Pass complex (i.e., not built-in types) by const-ref. This includes strings and Account types. For example, rather do
void Account::setName(const std::string& userName) ...
. Another possibility - if your compiler is recent enough - is to use std::string_view.Avoid throwing character strings and throw proper objects instead, see E.14.
You seem to have an invariant which says that the balance can never be negative. I would enforce this more consistently by writing a private function like
void checkInvariant() ...
that makes things likeassert(balance > 0)
(but be careful when comparing floating point values to constant values; it should be done with a threshold). Then, you can add this check to suitable functions and the invariant is nicely enforced by the class itself.The above can help you catch errors like in
withdraw
: is it acceptable that the amount becomes negative here?
$endgroup$
The class is doing too much: it is not the task of the
Account
class to also keep a database of all accounts. Remember: one class (or function), one responsibility. By the same principle,menuSelection
does too much in your main program.Use the initializer list in your constructor, i.e., do
Account::Account() : name_(), id_(0), balance(0)
. But even better, you should always try to avoid defining default operations, as per C.20 of the C++ Core Guidelines.Pass complex (i.e., not built-in types) by const-ref. This includes strings and Account types. For example, rather do
void Account::setName(const std::string& userName) ...
. Another possibility - if your compiler is recent enough - is to use std::string_view.Avoid throwing character strings and throw proper objects instead, see E.14.
You seem to have an invariant which says that the balance can never be negative. I would enforce this more consistently by writing a private function like
void checkInvariant() ...
that makes things likeassert(balance > 0)
(but be careful when comparing floating point values to constant values; it should be done with a threshold). Then, you can add this check to suitable functions and the invariant is nicely enforced by the class itself.The above can help you catch errors like in
withdraw
: is it acceptable that the amount becomes negative here?
edited 2 days ago
answered Apr 14 at 11:30
JuhoJuho
1,631712
1,631712
add a comment |
add a comment |
Thanks for contributing an answer to Code Review Stack Exchange!
- Please be sure to answer the question. Provide details and share your research!
But avoid …
- Asking for help, clarification, or responding to other answers.
- Making statements based on opinion; back them up with references or personal experience.
Use MathJax to format equations. MathJax reference.
To learn more, see our tips on writing great answers.
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f217415%2fsimulation-of-a-banking-system-with-an-account-class-in-c%23new-answer', 'question_page');
);
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown
Required, but never shown