3
\$\begingroup\$

I've been learning C++ for a while. I wrote this app sometimes ago. The feedback that I got for that app was quite helpful. Now I rewrote the app using the new knowledge that I acquired while learning. This app has been complied with MSVC std-c++14. I want to learn writing efficient code with as little memory usage as possible with zero try...catch block (if possible). Any suggestions on these aspects or any other area are appreciated.

bankdata.h

#pragma once

#include <string>
#include <fstream>

#include "defines.h"

class BankData
{
public:
    BankData();
    BankData(const char* name, const AccountType& acType, const long double& balance);
    BankData(const BankData& src) = default;
    BankData(BankData&& src) noexcept = default;
    BankData& operator=(const BankData& other) = default;
    BankData& operator=(BankData&& other) noexcept = default;
    ~BankData() = default;
    bool operator==(const unsigned long& acNum) const;
    bool operator==(const char* name) const;
    operator std::string() const;    
public:
    const char* getName() const;
    void setName(const char* name);
    AccountType getAcType() const;
    void setAcType(const AccountType& acType);
    long double getBalance() const;
    void setBalace(const long double& balace);
    unsigned long getAcNum() const;
    void setAcNum(const unsigned long& acNum);
    std::ofstream& prepareForStore(std::ofstream& ofs) const;
public:
    static unsigned long getAccountCount();
    static void setAccountCount(const unsigned long& count);
private:
    char m_name[MAX_STR_LEN];    
    AccountType m_acType;
    long double m_balance;
    unsigned long m_acNum;
private:
    static unsigned long sm_acNum;
};
 

BankData.cc

#include <cstring>
#include <sstream>

#include "bankdata.h"

unsigned long BankData::sm_acNum = 0UL;

BankData::BankData() 
    : m_acNum(0UL), m_name(""), m_acType(AccountType::CURRENT), m_balance(0.0L)
{
}

BankData::BankData(const char* name, const AccountType& acType, const long double& balance)
    : m_acType(acType), m_balance(balance)
{
    ++BankData::sm_acNum;
    this->m_acNum = BankData::sm_acNum;
    strcpy_s(this->m_name, MAX_STR_LEN, name);
}

bool BankData::operator==(const unsigned long& acNum) const
{
    return this->m_acNum == acNum;
}

bool BankData::operator==(const char* name) const
{
    return std::strcmp(this->m_name, name) == 0;
}

BankData::operator std::string() const
{
    std::ostringstream oss;
    oss << "Account Number:" << TAB << this->m_acNum << NEWLINE << DOUBLE_TAB
        << "Account Holder:" << TAB << this->m_name << NEWLINE << DOUBLE_TAB
        << "Account Type:" << TAB << accountTypeToString(this->m_acType) << NEWLINE << DOUBLE_TAB
        << "Account Balance" << TAB << this->m_balance;
    return oss.str();
}

const char* BankData::getName() const
{
    return this->m_name;
}

void BankData::setName(const char* name)
{
    strcpy_s(this->m_name, MAX_STR_LEN, name);
}

AccountType BankData::getAcType() const
{
    return this->m_acType;
}

void BankData::setAcType(const AccountType& acType)
{
    this->m_acType = acType;
}

long double BankData::getBalance() const
{
    return this->m_balance;
}

void BankData::setBalace(const long double& balace)
{
    this->m_balance = balace;
}

unsigned long BankData::getAcNum() const
{
    return this->m_acNum;
}

void BankData::setAcNum(const unsigned long& acNum)
{
    this->m_acNum = acNum;
}

std::ofstream& BankData::prepareForStore(std::ofstream& ofs) const // serialization
{
    ofs << this->m_acNum << ';' << this->m_name << '\0' << accountTypeToChar(this->m_acType)
        << ';' << this->m_balance;
    return ofs;
}

unsigned long BankData::getAccountCount()
{
    return BankData::sm_acNum;
}

void BankData::setAccountCount(const unsigned long& count)
{
    BankData::sm_acNum = count;
}
 

 

bankmanager.h

#pragma once

#include <vector>
#include <string>

#include "bankdata.h"
#include "defines.h"

class BankManager final
{
public:
    BankManager();
    BankManager(const BankManager& src) = delete;
    BankManager(BankManager&& src) noexcept = delete;
    BankManager& operator=(const BankManager& other) = delete;
    BankManager& operator=(BankManager&& other) noexcept = delete;
    ~BankManager() = default;
    operator std::string() const;
public:
    bool createNewAccount(const char* name, const AccountType& acType, const long double& balance);
    bool deleteAccount(const unsigned long& acNum, const char* name);
    bool updateBalance(const unsigned long& acNum, const char* name, const long double& balance);
    bool updateAccountInfo(const unsigned long& acNum, const char* name, const char* newName);
    bool updateAccountInfo(const unsigned long& acNum, const char* name, const AccountType& acType);
    bool updateAccountInfo(const unsigned long& acNum, const char* name, const char* newName, const AccountType* acType);
    bool accountExists(const unsigned long& acNum) const;
    bool accountExists(const char* name) const;
    const BankData& getBankData(const unsigned long& acNum) const;    
    BankData& getBankData(const unsigned long& acNum);
    bool isEmpty() const;
    std::string to_string() const;
    bool store() const;
    bool load();
private:    
    size_t indexOf(const unsigned long& acNum) const;
private:
    std::vector<BankData> m_bankInfo{};
};
 

BankManager.cc

#include <algorithm>
#include <utility>
#include <sstream>
#include <iomanip>
#include <fstream>
#include <cstdlib>

#include "bankmanager.h"
#include "defines.h"

static const char* storeFileName = "v8.dat";

bool BankManager::createNewAccount(const char* name, const AccountType& acType, const long double& balance)
{
    if (this->accountExists(name))
    {
        return false;
    }
    this->m_bankInfo.push_back(BankData(name, acType, balance)); // move constructor is called
    return true;
}

bool BankManager::deleteAccount(const unsigned long& acNum, const char* name)
{
    if (!this->accountExists(acNum) || !this->accountExists(name))
    {
        return false;
    }
    this->m_bankInfo.erase(this->m_bankInfo.begin() + this->indexOf(acNum));
    return true;
}

bool BankManager::updateBalance(const unsigned long& acNum, const char* name, const long double& balance)
{
    if (!this->accountExists(acNum) || !this->accountExists(name))
    {
        return false;
    }
    BankData& clientData = this->getBankData(acNum);
    long double curBalance = clientData.getBalance();
    curBalance += balance;
    clientData.setBalace(curBalance);
    return true;
}

bool BankManager::updateAccountInfo(const unsigned long& acNum, const char* name, const char* newName)
{
    return this->updateAccountInfo(acNum, name, newName, nullptr);
}

bool BankManager::accountExists(const char* name) const
{
    return std::find(this->m_bankInfo.begin(), this->m_bankInfo.end(), name) != this->m_bankInfo.end();
}

size_t BankManager::indexOf(const unsigned long& acNum) const
{
    return std::find(this->m_bankInfo.begin(), this->m_bankInfo.end(), acNum) - this->m_bankInfo.begin();
}

BankData& BankManager::getBankData(const unsigned long& acNum)
{
    size_t pos = this->indexOf(acNum);
    return this->m_bankInfo.at(pos);
}

bool BankManager::isEmpty() const
{
    return this->m_bankInfo.empty();
}

std::string BankManager::to_string() const
{
    return this->operator std::string();
}

bool BankManager::store() const // serialization
{
    std::ofstream ofs(storeFileName, std::ios::binary | std::ios::out); // not a good approach since we may have big chunk of data so we create file each time we invoke store()
    if (!ofs.good() || !ofs.is_open())                                    // what would be a better option?
    {
        return false;
    }
    ofs << this->m_bankInfo.size() << '*' << BankData::getAccountCount() << '*';
    for (const BankData& b : this->m_bankInfo)
    {
        b.prepareForStore(ofs) << '*';
    }
    ofs.close();
    return true;
}

bool BankManager::load() // deserialization; only call once at app initialization
{
    std::ifstream ifs(storeFileName, std::ios::in | std::ios::binary);
    if (!ifs.good() || !ifs.is_open())
    {
        return false;
    }
    std::string tmp;
    std::getline(ifs, tmp, '*'); // vSize
    size_t vSize = std::stoull(tmp);
    std::getline(ifs, tmp, '*'); // count
    unsigned long acCount = std::stoul(tmp);
    BankData::setAccountCount(acCount);
    for (size_t i = 0UL; i < vSize; i++)
    {
        std::getline(ifs, tmp, ';');
        acCount = std::stoul(tmp); // acNum, in this case
        BankData b;
        b.setAcNum(acCount);
        std::getline(ifs, tmp, '\0'); // name
        b.setName(tmp.c_str());
        std::getline(ifs, tmp, ';'); // acType
        AccountType acType = charToAccountType(tmp[0]);
        b.setAcType(acType);
        std::getline(ifs, tmp, '*'); // balance
        long double balance = std::stold(tmp);
        b.setBalace(balance);
        this->m_bankInfo.push_back(std::move(b)); // 'b' is not a r-value, so we cast it one
    }
    return true;
}

const BankData& BankManager::getBankData(const unsigned long& acNum) const
{
    //return const_cast<const BankData&>(this->getBankData(acNum)); // won't work since it's gonna cause overflow // return a const version of BankData&
    size_t pos = this->indexOf(acNum);
    return this->m_bankInfo.at(pos);
}

bool BankManager::updateAccountInfo(const unsigned long& acNum, const char* name, const AccountType& acType)
{
    return this->updateAccountInfo(acNum, name, nullptr, &acType);
}

bool BankManager::updateAccountInfo(const unsigned long& acNum, const char* name, const char* newName, const AccountType* acType)
{
    if (name == nullptr && acType == nullptr)
    {
        return false;
    }
    if (!this->accountExists(acNum) || !this->accountExists(name))
    {
        return false;
    }
    BankData& clientData = this->getBankData(acNum);
    if (newName != nullptr)
    {
        clientData.setName(newName);
    }
    if (acType != nullptr)
    {
        clientData.setAcType(*acType);
    }
    return true;
}

bool BankManager::accountExists(const unsigned long& acNum) const
{
    return std::find(this->m_bankInfo.begin(), this->m_bankInfo.end(), acNum) != this->m_bankInfo.end();
}

BankManager::BankManager()
{
    if (existsFile(storeFileName))
    {
        if (!this->load())
        {
            std::exit(EXIT_FAILURE);
        }
    }
}

BankManager::operator std::string() const
{
    std::ostringstream oss;
    for (size_t i = 0UL; i < this->m_bankInfo.size(); i++)
    {
        oss << this->m_bankInfo.at(i).getAcNum() << std::setw(28)
            << this->m_bankInfo.at(i).getName() << std::setw(25)
            << accountTypeToChar(this->m_bankInfo.at(i).getAcType()) << std::setw(39)
            << this->m_bankInfo.at(i).getBalance() << '\n';
    }
    return oss.str();
}
 

displayui.h

#pragma once

#include <iostream>
#include <vector>

class DisplayUI final
{
public:
    DisplayUI() = delete;
    explicit DisplayUI(void(*clearFunc)());
    DisplayUI(const DisplayUI& src) = delete;
    DisplayUI(DisplayUI&& src) noexcept = delete;
    DisplayUI& operator=(const DisplayUI& other) = delete;
    DisplayUI& operator=(DisplayUI&& other) noexcept = delete;
    ~DisplayUI() = default;
public:
    char mainMenu() const;
    void coverMenu() const;
    void accountStatusMenu(const std::string& accFormatStr) const;
    void allAccountMenu(const std::string& accListFormatStr) const;
    void clearScreen() const;
    void printTransectionHeader(const char* str) const;
    std::string& getHeadingMenuString(const char* menuName, char character, size_t len, std::string& out) const;
private:
    void (*m_clear)();
};

 

DisplayUI.cc

#include <cstring>
#include <string>
#include <conio.h> // _getch()
#include <iomanip>
#include <sstream>
#include <utility>

#include "displayui.h"
#include "defines.h"

char DisplayUI::mainMenu() const
{
    sleepFor(300);
    this->clearScreen();
    char options[] = "12345678";
    char ret = '\0';
    const char* menuHeaders[MENU_HEADER_ARRAY_LEN] = { "CREATE AN ACCOUNT", "DIPOSIT AMOUNT", "WITHDRAW AMOUNT", "BALANCE ENQUIRE", "VIEW LIST OF ALL ACCOUNT HOLDERS", "CLOSE AN ACCOUNT", "MODIFY AN ACCOUNT", "EXIT" };
    std::cout << CARRIAGE_RETURN << DOUBLE_NEWLINE << DOUBLE_TAB << "MAIN MENU" << DOUBLE_NEWLINE;
    for (size_t i = 0UL; i < MENU_HEADER_ARRAY_LEN; i++)
    {
        std::cout << DOUBLE_TAB << '0' << i + 1 << "  " << menuHeaders[i] << DOUBLE_NEWLINE;
    }
    do
    {
        std::cout << DOUBLE_NEWLINE << DOUBLE_TAB 
            << "Please select <1-8> to operate:" << DOUBLE_TAB;
        ret = _getch();
        size_t optionsLen = std::strlen(options);
        for (size_t i = 0UL; i < optionsLen; i++)
        {
            if (options[i] == ret)
            {
                return ret;
            }
        }
    } while (true);
}

void DisplayUI::coverMenu() const
{
    std::cout << TEN_NEWLINE << TRIPPLE_TAB
        << "Welcome to BankManager v2.0: A Banking Management System by Alex"
        << DOUBLE_NEWLINE << TRIPPLE_TAB << "Please Press 'Enter' To Continue..."
        << TRIPPLE_NEWLINE << TRIPPLE_TAB;
    std::cin.get();
    sleepFor(2000);
    this->clearScreen();
}

void DisplayUI::accountStatusMenu(const std::string& accFormatStr) const
{
    std::string temp;
    std::cout << CARRIAGE_RETURN << DOUBLE_NEWLINE << DOUBLE_NEWLINE
        << DOUBLE_TAB << this->getHeadingMenuString("ACCOUNT STATUS", '-', 5UL, temp)
        << TRIPPLE_NEWLINE << DOUBLE_TAB << accFormatStr << NEWLINE;
}

void DisplayUI::allAccountMenu(const std::string& accListFormatStr) const
{
    char heading[] = "List of All the Account Holders";
    size_t headingLen = std::strlen(heading);
    std::string space(headingLen * 3, '-');
    std::cout << TRIPPLE_NEWLINE << std::setw(headingLen * 2)
        << heading << std::setw(headingLen * 2) << DOUBLE_NEWLINE
        << space << NEWLINE << "Ac No." << std::setw(20) << "Name"
        << std::setw(30) << "Type" << std::setw(38) << "Amount"
        << NEWLINE << space << NEWLINE << accListFormatStr << space << NEWLINE;
}

void DisplayUI::clearScreen() const
{
    this->m_clear();
}

void DisplayUI::printTransectionHeader(const char* str) const
{
    std::cout << DOUBLE_NEWLINE << DOUBLE_TAB
        << "Enter the amount to " << str << ':' << TAB;
}

DisplayUI::DisplayUI(void(*clearFunc)()) : m_clear(clearFunc)
{
}

std::string& DisplayUI::getHeadingMenuString(const char* menuName, char character, size_t len, std::string& out) const
{
    std::ostringstream oss;
    oss << std::move(std::string(len, character)) << menuName << std::move(std::string(len, character));
    out = std::move(oss.str());
    return out;
}
 

bankmanagerapp.h

#pragma once

#include "bankmanager.h"
#include "displayui.h"

class BankManagerApp final
{
public:
    BankManagerApp() = delete;
    explicit BankManagerApp(void(*clearFunc)());
    BankManagerApp(const BankManagerApp& src) = delete;
    BankManagerApp(BankManagerApp&& src) noexcept = delete;
    BankManagerApp& operator=(const BankManagerApp& other) = delete;
    BankManagerApp& operator=(BankManagerApp&& other) noexcept = delete;
    ~BankManagerApp();
public:
    void run();
private:
    void createNewAccount();
    void updateBalance(const unsigned long& acNum, const char* name, bool deposit=true);
    void showAccountStatus(const unsigned long& acNum) const;
    void showAllAccounts() const;
    void updateAccountInfo(const unsigned long& acNum, const char* name);
    void deleteAccount(const unsigned long& acNum, const char* name);
    void getAccoutInfo(unsigned long& acNum, std::string& name) const;
private:
    BankManager* m_ptrBM;
    DisplayUI* m_ptrDisUI;
};

 

BankManagerApp.cc

#include <cctype>
#include <vector>
#include <utility>
#include <string>
#include <iostream>

#include "bankmanagerapp.h"
#include "defines.h"

BankManagerApp::BankManagerApp(void(*clearFunc)())
    : m_ptrBM(new BankManager()), m_ptrDisUI(new DisplayUI(clearFunc))
{
    // this may throw std::bad_alloc
    // how to overcome this without checking if these two
    // pointers are nullptr or not for each function call?
}

BankManagerApp::~BankManagerApp()
{
    delete this->m_ptrBM;
    delete this->m_ptrDisUI;
}

void BankManagerApp::run()
{
    this->m_ptrDisUI->coverMenu();
    bool continueProgram = true;
    while (continueProgram)
    {
        char userChoice = this->m_ptrDisUI->mainMenu();
        unsigned long acNum = 0UL;
        std::string name;
        switch (userChoice)
        {
        case '1':
            this->createNewAccount();
            break;
        case '2':
            this->getAccoutInfo(acNum, name);
            if (acNum != 0UL)
            {
                this->updateBalance(acNum, name.c_str());                
            }
            break;
        case '3':
            this->getAccoutInfo(acNum, name);
            if (acNum != 0UL)
            {
                this->updateBalance(acNum, name.c_str(), false);
            }
            break;
        case '4':
            this->getAccoutInfo(acNum, name);
            if (acNum != 0UL)
            {
                this->showAccountStatus(acNum);
                std::cin.ignore();
            }
            break;
        case '5':
            this->showAllAccounts();
            std::cin.ignore();
            break;
        case '6':
            this->getAccoutInfo(acNum, name);
            if (acNum != 0UL)
            {
                this->deleteAccount(acNum, name.c_str());
            }
            break;
        case '7':
            this->getAccoutInfo(acNum, name);
            if (acNum != 0UL)
            {
                this->updateAccountInfo(acNum, name.c_str());
            }
            break;
        case '8':
            continueProgram = false;
            this->m_ptrBM->store(); // we store any everything at the end of program; since a CLI app shouldn't be closed using 'X' button, we don't
            break;                  // need to store the data after each transaction or modification of the user data
        }                           // however, this is "not" a good idea since program can terminate for other reasons, so we won't get any unsaved data
    }                               // nonetheless, for this simple app I think it is okay
}

void BankManagerApp::createNewAccount()
{
    sleepFor(500);
    this->m_ptrDisUI->clearScreen();
    std::vector<std::string> accountInfo{};
    std::string temp;

    std::cout << CARRIAGE_RETURN << DOUBLE_NEWLINE << DOUBLE_NEWLINE
        << DOUBLE_TAB << this->m_ptrDisUI->getHeadingMenuString("Register New Account", '*', 5UL, temp);
    do
    {
        std::cout << NEWLINE << DOUBLE_TAB 
            << "Enter the Name of the Account Holder:" << DOUBLE_TAB;
        std::getline(std::cin, temp); // account name
        if (this->m_ptrBM->accountExists(temp.c_str()))
        {
            std::cout << NEWLINE << DOUBLE_TAB
                << "The person already has account!" << NEWLINE;
            sleepFor(1000);
            this->m_ptrDisUI->clearScreen();
        }
    } while (temp.compare("") == 0 || this->m_ptrBM->accountExists(temp.c_str()));
    accountInfo.push_back(std::move(temp));
    do
    {
        std::cout << NEWLINE << DOUBLE_TAB 
            << "Enter the Type <C/S/F>:" << DOUBLE_TAB << DOUBLE_TAB;
        std::getline(std::cin, temp); // account type
        if (temp[0] == 'C' || temp[0] == 'S' || temp[0] == 'F')
        {
            break;
        }
    } while (true);
    accountInfo.push_back(std::move(temp));
    do
    {
        std::cout << NEWLINE << DOUBLE_TAB 
            << "Enter the Amount:" << DOUBLE_TAB << DOUBLE_TAB;
        std::getline(std::cin, temp); // balance
    } while (temp.compare("") == 0 || !isFloat(temp));
    accountInfo.push_back(std::move(temp));
    if (!this->m_ptrBM->createNewAccount(
        accountInfo.at(0).c_str(),
        charToAccountType(accountInfo.at(1)[0]),
        std::stold(accountInfo.at(2))))
    {
        std::cout << "couldn't create account\n";
    }
    else
    {
        std::cout << DOUBLE_NEWLINE << TRIPPLE_TAB << "Record Updated!!!";
    }
}

void BankManagerApp::updateBalance(const unsigned long& acNum, const char* name, bool deposit)
{
    sleepFor(500);
    this->m_ptrDisUI->clearScreen();
    std::string temp;
    const char* msg = deposit ? "Deposit to Account" : "Withdraw from Account";
    long double balance = 0.0L;
    std::cout << TRIPPLE_NEWLINE << DOUBLE_TAB
        << this->m_ptrDisUI->getHeadingMenuString(msg, '*', 5UL, temp);
    const BankData& clientData = this->m_ptrBM->getBankData(acNum);
    temp = std::move(static_cast<std::string>(clientData));
    this->m_ptrDisUI->accountStatusMenu(temp);
    do
    {
        this->m_ptrDisUI->printTransectionHeader(deposit ? "DEPOSIT" : "WITHDRAW");
        std::getline(std::cin, temp);
    } while (temp.compare("") == 0 || !isFloat(temp));
    balance = std::stold(temp);
    if (!deposit)
    {
        balance *= -1L; // to make negative // balance -= balance * 2 -> make positive
    }
    if (!this->m_ptrBM->updateBalance(acNum, name, balance))
    {
        std::cout << "couldn't update balance\n";
    }
    else
    {
        std::cout << DOUBLE_NEWLINE << TRIPPLE_TAB << "Record Updated!!!";
    }
}

void BankManagerApp::showAccountStatus(const unsigned long& acNum) const
{
    sleepFor(500);
    this->m_ptrDisUI->clearScreen();
    const BankData& clientData = this->m_ptrBM->getBankData(acNum);
    std::string temp(std::move(static_cast<std::string>(clientData)));
    this->m_ptrDisUI->accountStatusMenu(temp);
}

void BankManagerApp::showAllAccounts() const
{
    sleepFor(500);
    this->m_ptrDisUI->clearScreen();
    std::string temp(std::move(this->m_ptrBM->to_string()));
    this->m_ptrDisUI->allAccountMenu(temp);
}

void BankManagerApp::updateAccountInfo(const unsigned long& acNum, const char* name)
{
    sleepFor(500);
    this->m_ptrDisUI->clearScreen();
    std::string temp;
    const char* options[2] = { "Account holder's name", "Account type" };
    std::cout << TRIPPLE_NEWLINE << DOUBLE_TAB
        << this->m_ptrDisUI->getHeadingMenuString("Modify An Account Information", '*', 5UL, temp);
    for (int i = 0; i < 2; i++)
    {
        std::cout << DOUBLE_NEWLINE << DOUBLE_TAB
            << "Please press " << i + 1 << " to edit "
            << options[i] << NEWLINE;
    }
    int userChoice = 0;
    do
    {
        std::cout << DOUBLE_NEWLINE << DOUBLE_TAB << ROOT_SYMBOL;
        std::getline(std::cin, temp);
        if (temp[0] == '1' || temp[0] == '2')
        {
            break;
        }
    } while (true);
    userChoice = temp[0] - '0';
    switch (userChoice)
    {
    case 1:
        {
            do
            {
                std::cout << DOUBLE_NEWLINE << DOUBLE_TAB
                    << "Please enter new name:" << TAB;
                std::getline(std::cin, temp);
            } while (temp.compare("") == 0);
        }
        break;
    case 2:
        {
            do
            {
                std::cout << DOUBLE_NEWLINE << DOUBLE_TAB
                    << "Enter enter new account type <C/S/F>:" << DOUBLE_TAB << DOUBLE_TAB;
                std::getline(std::cin, temp);
                if (temp[0] == 'C' || temp[0] == 'S' || temp[0] == 'F')
                {
                    break;
                }
            } while (true);
        }
        break;
    default:
        return;
    }
    std::cout << DOUBLE_NEWLINE << DOUBLE_TAB
        << "Press \"M\" to modify again" << NEWLINE << DOUBLE_TAB
        << "Press \"S\" save and exit" << NEWLINE << DOUBLE_TAB
        << "Press \"N\" to exit without saving"; 
    char userResponse = '\0';
    std::string temp2; // is there a way to avoid this? since I can't use 'temp' here which is holding value we need later
    do
    {
        std::cout << NEWLINE << DOUBLE_TAB << ROOT_SYMBOL;
        std::getline(std::cin, temp2);
        if (temp2[0] == 'M' || temp2[0] == 'S' || temp2[0] == 'N')
        {
            break;
        }

    } while (true);
    userResponse = temp2[0];
    switch (userResponse)
    {
    case 'M':
        this->updateAccountInfo(acNum, name);
        break;
    case 'S':
        {
            bool res = userChoice == 1
                ? this->m_ptrBM->updateAccountInfo(acNum, name, temp.c_str())
                : this->m_ptrBM->updateAccountInfo(acNum, name, charToAccountType(temp[0]));
            if (!res)
            {
                std::cout << "couldn't update account information\n";
            }
            else
            {
                std::cout << DOUBLE_NEWLINE << TRIPPLE_TAB << "Record Updated!!!";
            }
        }
        break;
    default:
        return;
    }
}

void BankManagerApp::deleteAccount(const unsigned long& acNum, const char* name)
{
    sleepFor(500);
    this->m_ptrDisUI->clearScreen();
    if (!this->m_ptrBM->deleteAccount(acNum, name))
    {
        std::cout << "couldn't delete account\n";
    }
    else
    {
        std::cout << DOUBLE_NEWLINE << TRIPPLE_TAB << "Record Updated!!!";
    }
}

void BankManagerApp::getAccoutInfo(unsigned long& acNum, std::string& name) const
{
    sleepFor(500);
    this->m_ptrDisUI->clearScreen();
    std::string tmp;
    do
    {
        std::cout << DOUBLE_NEWLINE << DOUBLE_TAB
            << "Please enter the account number:" << DOUBLE_TAB;
        std::getline(std::cin, tmp);
    } while (tmp.compare("") == 0 || !isDigit(tmp));
    acNum = std::stoul(tmp);
    do
    {
        std::cout << DOUBLE_NEWLINE << DOUBLE_TAB
            << "Please enter the account holder's name:" << DOUBLE_TAB;
        std::getline(std::cin, name);
    } while (name.compare("") == 0);
    if (!this->m_ptrBM->isEmpty())
    {
        if (!this->m_ptrBM->accountExists(acNum) || !this->m_ptrBM->accountExists(name.c_str()))
        {
            acNum = 0UL;
            name.clear();
            std::cout << NEWLINE << DOUBLE_TAB
                << "Not found!" << NEWLINE;
            sleepFor(1000);
        }
    }
    else
    {
        acNum = 0UL;
        name.clear();
        std::cout << NEWLINE << DOUBLE_TAB
            << "No account has been created!\n";
        sleepFor(1000);
    }
}
 

defines.h

#pragma once

#include <chrono>
#include <thread>
#include <cctype>
#include <regex>
#include <fstream>

constexpr size_t MAX_STR_LEN = 30UL;
constexpr size_t MENU_HEADER_ARRAY_LEN = 8UL;
constexpr const char* DOUBLE_NEWLINE = "\n\n";
constexpr char NEWLINE = '\n';
constexpr char TAB = '\t';
constexpr const char* TEN_NEWLINE = "\n\n\n\n\n\n\n\n\n\n";
constexpr const char* TRIPPLE_NEWLINE = "\n\n\n";
constexpr const char* DOUBLE_TAB = "\t\t";
constexpr const char* TRIPPLE_TAB = "\t\t\t";
constexpr char CARRIAGE_RETURN = '\r';
constexpr const char* ROOT_SYMBOL = ">>> ";

enum class AccountType
{
    CURRENT,
    SAVINGS,
    FIXED
};

static void sleepFor(unsigned long milSec)
{
    std::this_thread::sleep_for(std::chrono::milliseconds(milSec));
}

static AccountType charToAccountType(char c)
{
    return c == 'C' ? AccountType::CURRENT
        : c == 'S' ? AccountType::SAVINGS
        : AccountType::FIXED;
}

static char accountTypeToChar(const AccountType& accType)
{
    return accType == AccountType::CURRENT ? 'C' 
        : accType == AccountType::SAVINGS ? 'S'
        : 'F';
}

static const char* accountTypeToString(const AccountType& accType)
{
    return accType == AccountType::CURRENT ? "CURRENT"
        : accType == AccountType::SAVINGS ? "SAVINGS"
        : "FIXED";
}

static bool isDigit(const std::string& str)
{
    std::string::const_iterator cit = str.cbegin();
    while (cit != str.end() && std::isdigit(*cit))
    {
        ++cit;
    }
    return !str.empty() && cit == str.end();
}

static bool isFloat(const std::string& str)
{
    std::regex pattern("[0-9]*\\.?[0-9]+$");
    return std::regex_match(str, pattern);
}

static bool existsFile(const char* name)
{
    std::ifstream ifs(name);
    return ifs.good();
}
 

main.cc

#if !defined(WIN32_LEAN_AND_MEAN)
#define WIN32_LEAN_AND_MEAN
#endif
#include <Windows.h>

#include "bankmanagerapp.h"

static void clear() // os dependent 
{
    COORD topLeft{ 0, 0 };
    HANDLE console = ::GetStdHandle(STD_OUTPUT_HANDLE);
    CONSOLE_SCREEN_BUFFER_INFO screen;
    DWORD written;
    ::GetConsoleScreenBufferInfo(console, &screen);
    ::FillConsoleOutputCharacterA(console, ' ', screen.dwSize.X * screen.dwSize.Y, topLeft, &written);
    ::FillConsoleOutputAttribute(console, FOREGROUND_GREEN | FOREGROUND_RED | FOREGROUND_BLUE, screen.dwSize.X * screen.dwSize.Y, topLeft, &written);
    ::SetConsoleCursorPosition(console, topLeft);
}

int main(int argc, const char* argv[])
{
    BankManagerApp app(clear);
    app.run();
    return 0;
} 
\$\endgroup\$
3
  • \$\begingroup\$ Do you have any reason for not using std::string in bankdata.h \$\endgroup\$ – theProgrammer Nov 30 '20 at 19:20
  • \$\begingroup\$ @theProgrammer I could use const std::string& but I read somewhere (don't remember the source right now), that they could create temporary r-values. Correct me if I'm wrong. \$\endgroup\$ – A J Nov 30 '20 at 19:34
  • \$\begingroup\$ @AJ yes, use string_view. \$\endgroup\$ – Aryan Parekh Dec 1 '20 at 5:09
2
\$\begingroup\$

Unnecessay *this

The use of *this is both unnecessary and reductant. when you call a member function with an object of the class, it implicitly calls it with the object as its first argument, this can simply be explained as thus BankData(*this, args)

so the extra-qualification just makes the code unreadable and adds no gain.

Prefer std::string to const char*

const char* pointer is error-prone, I can't seem to understand why you are favoring error-proned const char* to std::string, your BankData constructor copies elements of the argument name to m_name which could have been simply written as m_name = name

No need for a destructor

BankData includes a destructor which is unnecessary, a destructor is only needed when you acquire a resource e.g lock, memory etc. Your class doesn't do any of those so no need of declaring it, also copy assignment and move assignment are not needed, let the compiler do the job for you.

Overload operator<< instead of calling std::string:

I tried calling operator<< for BankData class only for me to realize it was not implemented, you alternatively implemented std::string(). I believe std::cout << my_bankdata is more elegant and simple to understand than std::string(my_bankdata)

reduce your setters and getters

AccountType is currently an enum class, you don't need a setter and getter here, setters and getters are mostly used when you want to protect the class from invalid data, in this case, setters and getters should be removed, since users of the class would only initialize the account type with members of the enum class.

As a side note: Instead of checking both account number and account name, define operator== that compares two bankdata.

Too many indirection

getBankData(acNum) calls indexOf(accNum), if you defined an operator==, getBankData can be found easily with std::find. Note std::find returns an iterator pointing to the element or off-the-end iterator if it wasn't found.

Instead of set and get for balance data member, have a function balance that returns a reference and const reference to balance. Updating the balace would simply be clientData.balance += bal

\$\endgroup\$
1
\$\begingroup\$

In general in the new versions of c++, to be more precise on 17, const char* can be changed to std::string_view. Is an easy change and your code will be more up to date.

\$\endgroup\$
3
  • \$\begingroup\$ Thanks. This could be an option but didn't want to upgrade to std-c++17 just for this only. \$\endgroup\$ – A J Nov 30 '20 at 19:38
  • \$\begingroup\$ @AJ What are the downsides of upgrading to C++17? Moreover, using string_view is safer. \$\endgroup\$ – Aryan Parekh Dec 1 '20 at 5:07
  • \$\begingroup\$ This is a good point, but if you add why using string_view would be better, your answer will be better. \$\endgroup\$ – Aryan Parekh Dec 1 '20 at 5:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.