БЛОГ НА HUSL


Оцените качество кода онлайн банка с 3 функциями

Автор вопроса: 19g9nd

Изучаю С++ 3 недели, хочу узнать как можно улучшить мой код. Можете, пожалуйста, оценить этот вариант? Он не докончен, но хочется узнать в каком направлении стоит двигаться. Приложение выполняет роль онлайн банка с 3 функциями (произвести оплату, трансфер на карту, отчёт).

    #include <iostream>

using namespace std;


int communIndex = 0, mobIndex = 0, otherIndex = 0;
double balance = 100;


void OptionMessage();

int MakePayment(double arr1[], double arr2[], double arr3[], const int length);

int CommunalPayment(double arr[], const int length);

int MobilePayment(double arr[], const int length) { return 0; }

int OtherPayment(double arr[], const int length) { return 0; }

void GenerateReceipt(int serviceType, double sum);


int main()
{
    const int size = 50;

    double communalArr[size]{};
    double mobileArr[size]{};
    double otherArr[size]{};


    enum mainOpt { EXIT, PAYMENT, TRANSFER, REPORT };
    enum mobOpt{BAKCELL, AZERCELL};
    enum otherOpt{DONATE};

    int userChoice;

    OptionMessage();
    cin >> userChoice;


    while (userChoice)
    {
        switch (userChoice)
        {
        case EXIT:
            userChoice = 0;
            break;
        case PAYMENT:
            return MakePayment(communalArr, mobileArr, otherArr, size);
            break;
        case TRANSFER:
            int SummTransfer;
            int CardNum;
            cout << "Enter transfer card number: " << endl;
            cin >> CardNum;
            if (CardNum < 10000000000000000) cout << "Wrong card number lenght!"; //проверка длины! error
            break;
        case REPORT:
            break;
        default:
            break;
        }

    }
}


void OptionMessage() {//старт меню
    cout << "\n=============OPTIONS=============\n";
    cout << "1.Send Money" << endl;
    cout << "2.Transfer" << endl;
    cout << "3. Checkout" << endl;
    cout << "4. Logout" << endl;

    cout << "Your choice: ";//выбор пользователя
}


int MakePayment(double commArr[], double mobArr[], double otherArr[], const int length) {

    int paymentChoice;//если выбор оплата, то ->
    enum paymentOpt { EXIT, COMMUNAL, MOBILE, OTHER };

    cout << "\n=============OPTIONS=============\n"
        << "\t1 -> COMMUNAL\n"
        << "\t2 -> MOBILE\n"
        << "\t3 -> OTHER\n"
        << "\t0 -> EXIT\n\n";

    cout << "Your choice: ";

    cin >> paymentChoice;

    switch (paymentChoice)
    {
    case EXIT:
        return 0;
        break;
    case COMMUNAL:
        return CommunalPayment(commArr, length);
        break;
    case MOBILE:
        return MobilePayment(mobArr, length);
        break;
    case OTHER:
        return OtherPayment(otherArr, length);
        break;
    default:
        cout << "NO SUCH OPTION";
        return 1;
        break;
    }

}


int CommunalPayment(double arr[], const int length) {
    int comChoice;
    int abonentCode = 0;
    cout << "Enter abonent code" << endl;
    cin >> abonentCode;
    if (abonentCode < 10000000 && abonentCode >!9999999) { cout << "Wrong code lenght!"; }//проверка длины кода абонента
    cout << "Enter summ: \n";
    double sum;
    cin >> sum;
    if (sum < 1)cout << "Summ is too small!";
    enum communOpt { GAS = 1, WATER };

    cout << "\n=============OPTIONS=============\n"
        << "\t1 -> GAS\n"
        << "\t2 -> WATER\n"
        << "\t0 -> EXIT\n\n";

    cout << "Your choice: ";

    cin >> comChoice;

    switch (comChoice)
    {
    case GAS:
        cout << "ABONENT CODE: ";
        cin >> abonentCode;

        cout << "AMMOUNT: ";
        cin >> sum;

        if (abonentCode > 9999999 && abonentCode < 100000000 && sum <= balance) {
            balance -= sum;
            arr[communIndex] = sum;
            communIndex++;
            GenerateReceipt(0, sum);
            return 1;
        }
        return 0;
        break;
    case WATER:
        break;
    default:
        break;
    }
}


void GenerateReceipt(int serviceType, double amount) {
    enum serviceOpt { gas, water, azercell, bakcell, donate, transfer };
    static int ID = 1;
    cout << "RECEIPT #" << ID << endl
        << "DATE: " << "11/07/2022\n"
        << "TYPE: ";

    switch (serviceType)
    {
    case gas:
        cout << "GAS\n";
        break;
    case water:
        cout << "WATER\n";
        break;
    case azercell:
        cout << "AZERCELL\n";
        break;
    case bakcell:
        cout << "BAKCELL\n";
        break;
    case donate:
        cout << "DONATE\n";
        break;
    case transfer:
        cout << "TRANSFER\n";
        break;
    default:
        cout << "UNKNOWN\n";
        break;
    }

    cout << "AMOUNT: " << amount << endl << endl;
}

Источник

Ответы (2 шт):

Автор решения: Harry

Немного пройдусь по вашей

void GenerateReceipt(int serviceType, double amount);

Смотрите — вы передаете int serviceType, а только потом определяете enum serviceOpt. Вы уверены, что где-то концепция не поменяется и не нарушится соответствие значений переданного параметра и перечислений? Может, имеет смысл перечисление тогда делать глобальным?

А если нет — какой в нем вообще смысл? Для вывода строки? (кстати, непонятен смысл и неизменного и статического ID. Вероятно, это счетчик, который должен при каждом вызове расти? Да и жестко прошитая дата вызывает сомнения...

Ну, с датой разберётесь сами, а все остальное легким движением руки превращается в

void GenerateReceipt(int serviceType, double amount)
{
    const char* serviceName[] =
    {
        "GAS", "WATER", "AZERCELL",
        "BAKCELL","DONATE","TRANSFER","UNKNOWN"
    };

    if (serviceType >= size(serviceName))
        serviceType = size(serviceName) - 1;

    static int ID = 1;

    cout << "RECEIPT #" << ID++
         << "\nDATE: " << "11/07/2022"
         << "\nTYPE: " << serviceName[serviceType]
         << "\nAMOUNT: " << amount << "\n\n";
}
→ Ссылка
Автор решения: DmitryK

Если про ошибки - @Harry уже написал.
Также - для баланса лучше использовать целочисленную арифметику. Т.е. сумма на счете может быть только целая (например в копейках). И перевод можно сделать только в целых. Т.е. если у вас на счете 125,31 руб, значит у вас 12531 копеек. А для процентов существуют заранее определенные банковские правила округления.
Ещё ошибка - при выборе PAYMENT совершится платеж и main завершится. Здесь оператор return лишний.

case PAYMENT:
    return MakePayment(communalArr, mobileArr, otherArr, size);
    break;

А в остальном - никогда нет предела совершенству! :)

  • Если вы ставите тег c++, то наверное нужно использовать классы? А у вас программа на с. От с++ - только использование потоков ввода/вывода вместо prinf()/scanf(). У вас есть как минимум 1 сущность - счет клиента. А еще можно сделать сущность банк (или процессинг) - в нем будут методы произвести оплату,трансфер на карту,отчёт. Если отчет - это лог операций по счету, то его тоже можно сделать отдельной сущностью.
  • Использование идиомы RAII (https://habr.com/ru/sandbox/21603/). Все ресурсы инкапсулируются в классах. Как минимум счет клиента нужно инкапсулировать, чтобы к нему не было доступа помимо встроенных методов.
  • Глобальные переменные - зло. Особенно когда баланс глобальная переменная, которую может менять кто угодно и как угодно из любого места программы. При следовании RAII глобальные переменные попадут внутрь класса.
  • Написанные вами функции, если будут методами класса, будут обращаться к данным внутри класса, и в них не нужно будет передавать длинные списки параметров.
  • Аналогично - enum. У вас есть энумы, а вы по-прежнему передаете в функции в качестве параметров целые числа, а потом сравниваете с энумами. Передавайте сразу значения энума - это механизм контроля ошибок и улучшения читаемости кода. И не экономьте на имени enum. Для примера GenerateReceipt(int serviceType, double amount) меняете на GenerateReceipt(enum ServiceOption serviceType, double amount) - т.е. вы сразу передаете в функцию значение из enum ServiceOption - компилятор постарается вас уберечь от ошибки. Про генерацию строк из enum @Harry уже написал.
  • Зачем хранить в массивах суммы по отдельным коммунальным платежам double communalArr[size]{}; double mobileArr[size]{}; double otherArr[size]{};? Скорее всего вам нужен 1 лог по аккаунту, в котором будет учтено на какую цель пошел платеж.
  • Желательно отделить функции интерфейса (которые выводят меню, запрашивают выбор пользователя) от банковских функций.
    Ну и т.д.
#include <iostream>
#include <vector>
using namespace std;

enum TypeOfPayment { Income, Gas, Water, Azercell, Bakcell, Donate, Transfer, Exit};
const char* PaymentName[] =
    {
        "INCOME", "GAS", "WATER", "AZERCELL",
        "BAKCELL","DONATE","TRANSFER","UNKNOWN"
    };

enum AccountError { AllOK, NotEnoughFunds, Unknown }; // Ошибки при платежах
const char* AccountErrorName[] =
    {
        "Operation is OK!",
        "Not enough funds on user account!",
        "Unknown error!"
    };
// -------------------------------------------------------------------------- 
struct AccountLogRecord
{
    unsigned ID;  // можно вообще не добавлять - считать ID номер записи в векторе
    string RecDate; // или использовать std::chrono::year_month_day
    TypeOfPayment PaymentType;
    unsigned Amount;
    // string Comment;
    // string DetailsPayee; // реквизиты получателя платежа
    
    AccountLogRecord();
};

AccountLogRecord::AccountLogRecord()
{
    ID = 0;
    RecDate = "11.07.2022";
    PaymentType = TypeOfPayment::Income;
    Amount = 0;
}

// вывод записи лога в поток
std::ostream& operator<< (std::ostream &out, const AccountLogRecord& record) 
{
    cout << "RECEIPT #" << record.ID
         << "\tDATE: " << record.RecDate
         << "\tTYPE: " << PaymentName[record.PaymentType]
         << "\tAMOUNT: " << record.Amount << "\n";
    return cout;
}

// --------------------------------------------------------------------------

class UserAccount
{
    private:
    unsigned Balance;
    std::vector<AccountLogRecord> Log;
    
    public:
    UserAccount() : Balance (0) {};

    unsigned GetBalance(void) const { return Balance; }
    
    AccountError Increase(unsigned amount) 
    {  // можно добавить проверку на переполнение
        Balance+=amount; 
        return AccountError::AllOK; 
    }
    
    AccountError Decrease(unsigned amount) 
    { 
        if( Balance < amount ) 
            return AccountError::NotEnoughFunds;
        Balance-=amount; 
        return AccountError::AllOK;
    }

    const AccountLogRecord& GetLastLogRecord() const { return Log.back(); }
    const AccountLogRecord& GetLastLogRecord(size_t num) const { return Log.at(num); }
    void PrintLog(void) const
    {
        for(auto &i : Log)
            cout << i;
    }

    void AddToLog(AccountLogRecord&& rec) 
    {
        rec.ID = Log.size();
        Log.push_back(rec);
    }
};

// --------------------------------------------------------------------------

class BankProcessing
{
    private:
    UserAccount Account;
    
    unsigned GetAmount(void) const;
    unsigned GetAbonentCode(void) const;
    long     GetCardNumber(void) const;

    void OperationMessage(AccountError mess) const { cout << "Result of operation: " << AccountErrorName[mess] << "\n";}

    //Функции интерфейса - выводят меню и возвращают тип платежа
    TypeOfPayment ShowPaimentMenu(void) const;
    TypeOfPayment ShowCommunalMenu(void) const;
    TypeOfPayment ShowCheckoutMenu(void) const;
    
    public:
    unsigned Increase(unsigned amount) noexcept;
    unsigned MakePayment(unsigned amount, TypeOfPayment pay) noexcept;

    TypeOfPayment ShowMainMenu(void);
};

unsigned BankProcessing::GetAmount(void) const
{  // проверка корректности введения суммы 
    unsigned amount;
    cout << "\nEnter amount: ";
    while(true)
    {
        cin >> amount;
        if (amount > 1 && amount < 100000)
            return amount;
        cout << "Error! Amount must be in range 1 < amount < 100000.\n Enter valid amount:";
    }
}

unsigned BankProcessing::GetAbonentCode(void) const
{  // проверка корректности введения кода абонента
    unsigned abonentCode = 0;
    cout << "\nEnter abonent code from 7 digits: ";
    while(true)
    {
        cin >> abonentCode;
        if (abonentCode >=1000000 && abonentCode < 10000000)
            return abonentCode;
        cout << "Error! Wrong code lenght!.\n Enter valid abonent code from 7 digits:";
    }
}

long BankProcessing::GetCardNumber(void) const
{  // проверка корректности введения номера карты
    long CardNum;
    cout << "Enter transfer card number from 16 digits: " << endl;
    while(true)
    {
        cin >> CardNum;
        if (CardNum > 1000000000000000 && CardNum < 10000000000000000) // > 1000'0000'0000'0000  < 1'0000'0000'0000'0000)  
            break;
        cout << "Wrong card number lenght!\n Enter valid transfer card number from 16 digits: ";
    }
    return CardNum;
}

unsigned BankProcessing::Increase(unsigned amount) noexcept
{
    AccountError Res = Account.Increase(amount);
    
    AccountLogRecord tmp;
    tmp.Amount = amount;

    Account.AddToLog( std::move(tmp) );
    OperationMessage( AccountError::AllOK );
    cout << Account.GetLastLogRecord();
    return Account.GetBalance();
}
    
unsigned BankProcessing::MakePayment(unsigned amount, TypeOfPayment pay) noexcept
{
    AccountError Result = Account.Decrease(amount);
    if(Result != AccountError::AllOK)
    {
        cout << "Error!\n";
        OperationMessage( Result );
        return Account.GetBalance();
    }

    AccountLogRecord tmp;
    tmp.Amount = amount;
    tmp.PaymentType = pay;
    
    Account.AddToLog( std::move(tmp) );
    OperationMessage( Result );
    cout << Account.GetLastLogRecord();
    return Account.GetBalance();
}

TypeOfPayment BankProcessing::ShowMainMenu(void)
{
    enum MainMenuOption { EXIT, PAYMENT, TRANSFER, REPORT };
    
    while(true)
    {
        cout << "\n\n\n=============OPTIONS=============\n"
        << "1. Send Money\n"
        << "2. Transfer\n"
        << "3. Checkout\n"
        << "0. Logout\n"
        << "Your choice: ";//выбор пользователя
    
        int userChoice;

        while (true)
        {
            cin >> userChoice;
            if(userChoice >= MainMenuOption::EXIT && userChoice <= MainMenuOption::REPORT) // если ввод корректный - выход из цикла
                break;
            cout << "Wrong choice!\n Enter your choice again: ";
        }    
        
        TypeOfPayment TypeP = TypeOfPayment::Exit;
        
        switch (userChoice)
        {
            case MainMenuOption::EXIT : return TypeOfPayment::Exit; // выход из главного меню и программы
            case MainMenuOption::PAYMENT : TypeP = ShowPaimentMenu(); break;
            case MainMenuOption::TRANSFER : TypeP = TypeOfPayment::Transfer; break;
            case MainMenuOption::REPORT : Account.PrintLog(); break;
        }

        if(TypeP == TypeOfPayment::Exit) // выход из подменю в главное меню
            continue;
        
        long CardNumber = 0;
        // обработка платежа        
        switch(TypeP) // TypeOfPayment { Income, Gas, Water, Azercell, Bakcell, Donate, Transfer, Exit};
        {
            case TypeOfPayment::Gas :
            case TypeOfPayment::Water : GetAbonentCode(); break;
            case TypeOfPayment::Transfer : CardNumber = GetCardNumber(); break;
            case TypeOfPayment::Azercell :
            case TypeOfPayment::Bakcell :
            case TypeOfPayment::Donate : continue; // заглушка - переход на следующую итерацию цикла
        };
        unsigned Amount = GetAmount();
        MakePayment( Amount, TypeP);
    }
}

// отображает меню и возвращает тип платежа
TypeOfPayment BankProcessing::ShowPaimentMenu(void) const
{
    enum PaimentMenuOption { EXIT, COMMUNAL,  MOBILE, OTHER };
    TypeOfPayment TypeP = TypeOfPayment::Exit;
    
    int paymentChoice;//если выбор оплата, то ->
    cout << "\n=============OPTIONS=============\n"
        << "\t1 -> COMMUNAL\n"
        << "\t2 -> MOBILE\n"
        << "\t3 -> OTHER\n"
        << "\t0 -> EXIT\n"
        << "Your choice: ";
        
    while (true)
    {
        cin >> paymentChoice;
        if(paymentChoice >= PaimentMenuOption::EXIT && paymentChoice <= PaimentMenuOption::OTHER) // если ввод корректный - выход из цикла
            break;
        cout << "Wrong choice!\n Enter your choice again: ";
    }    

    switch (paymentChoice)
    {
        case PaimentMenuOption::EXIT : return TypeOfPayment::Exit; // выход в главное меню
        case PaimentMenuOption::COMMUNAL: return ShowCommunalMenu();
        case PaimentMenuOption::MOBILE: 
        case PaimentMenuOption::OTHER: return TypeOfPayment::Exit;
    };
    return TypeOfPayment::Exit;
}

TypeOfPayment BankProcessing::ShowCommunalMenu(void) const
{
    enum CommunalMenuOption { EXIT, GAS, WATER };
    cout << "\n=============OPTIONS=============\n"
        << "\t1 -> GAS\n"
        << "\t2 -> WATER\n"
        << "\t0 -> EXIT\n\n";
    
    int paymentChoice;
    while (true)
    {
        cin >> paymentChoice;
        if(paymentChoice >= CommunalMenuOption::EXIT && paymentChoice <= CommunalMenuOption::WATER) // если ввод корректный - выход из цикла
            break;
        cout << "Wrong choice!\n Enter your choice again: ";
    }    
    
    switch (paymentChoice)
    {
        case CommunalMenuOption::EXIT : return TypeOfPayment::Exit;
        case CommunalMenuOption::GAS : return TypeOfPayment::Gas;
        case CommunalMenuOption::WATER : return TypeOfPayment::Water;
    }
    return TypeOfPayment::Exit;
}

int main()
{
    BankProcessing BankProcess;
    BankProcess.Increase(100);
    BankProcess.ShowMainMenu();
}
→ Ссылка

licensed under cc by-sa 3.0 with attribution.