Прошу оценить код 3

Прошу оценить код 3

Учел все замечания в прошлом посте http://code-live.ru/forum/cpp/310/, конкретно:

  • каждая функция — логический блок;
  • глобальных переменных (кроме констант) в коде нет;
  • все переменные передаются в функции;
  • выход из программы один;
  • максимально использовал конкатенацию строкового вывода;

(Черепу)

  • от switch-а в slct_month() ушел и отправил его в knockdown вместе с расСчетом :)))

В общем вот код, как всегда буду рад любому комментарию (за них всегда СПАСИБО):

#include <iostream>
#include <string>
#include <cstdlib>
#include <iomanip>

using namespace std;

/*********************** ---------- ОПРЕДЕЛЕНИЯ СТРУКТУР ----------- ***************************/
struct data{
    int month;
    int choice;
    int current;
    int previous;
};
struct pabServ{
    double gas;
    double light;
    double water;
    double sum;
};
/******************************---------КОНСТАНТЫ ---------*************************************/
const string months[] =
{"январь", "февраль", "март", "апрель", "май", "июнь",
"июль", "август", "сентябрь", "октябрь", "ноябрь", "декабрь"};
const string type_calc[] = {"газа", "электричества", "воды"};
const double price1 = 0.7254;           // цена на газ
const double price2 = 0.2802;           // цена на воду
const double price3 = 3.41;             // цена на свет

/*********************** --------- ПРОТОТИПЫ ФУНКЦИЙ ----------- *******************************/
int MonthMenu();        // прототип функции для выбора месяца
int AccountMenu(data input);    // прототип функции вывода главного меню
double Gas(data input, const string type[]);        // прототип функции для вычисления расхода газа
double Light(data input, const string type[]);  // прототип функции для вычисления расхода света
double Water(data input, const string type[]);  // прототип функции для вычисления расхода воды
int InputChoice();          // прототип функции проверки корректного ввода
int InputReadout(data input, const string type[]);  // прототип функции для ввода паказаний
double TotalSum(pabServ total, data input); // прототип функции для подсчета и вывода общ. результата

int main()
{
    setlocale(LC_ALL, "Russian");

    data input = {0, 0, 0, 0};
    pabServ total = {0, 0, 0, 0};
    input.month = MonthMenu();
    do{
        if (0 == input.month)
            break;
        system("cls");
        input.choice = AccountMenu(input);
        switch(input.choice){
        case 1 :
            system("cls");
            total.gas = Gas(input, type_calc);
            break;
        case 2 :
            system("cls");
            total.light = Light(input, type_calc);
            break;
        case 3 :
            system("cls");
            total.water = Water(input, type_calc);
            break;
        case 4 :
            system("cls");
            total.sum = TotalSum(total, input);
            break;              
        case 5 :
// при выборе месяца все расчеты сбрасываются на 0
            system("cls");
            total.sum = total.water = total.light = total.gas = 0;
            input.month = MonthMenu();
            break;
        }
    }while (input.choice != 0);
    system("cls");
    cout << "До свидания!\n";
    return 0;
}
int MonthMenu()
{
    int choice;
    do{
        cout << "Выберите месяц:\n\n";
        for (int i = 0; i < 6; ++i){
            cout << setw(2) << right << (i + 1) << ". " << setw(10) << left << months[i]
            << setw(2) << right << (i + 7) << ". " << setw(10) << left << months[i + 6] << endl;
        }
        cout << setw(16) << right << "0. Выход\n\n"
            << "Ваш выбор: ";
        choice = InputChoice();
        if (choice > 12){
            system("cls");
            cout << "Выберите пункт меню >>> \n\n";
        }
    }while (choice > 12);
    return choice;
}
int AccountMenu(data in)
{
    int choice;
    do{
        cout << "Расчет оплаты коммунальных услуг за " << months[in.month - 1] << " месяц\n"
            "\n1. Расчет оплаты за газ\n"
            "2. Расчет оплаты за электричество\n"
            "3. Расчет оплаты за воду\n"
            "4. Общая сумма к оплате за " << months[in.month - 1] << "\n"
            "5. Выбор месяца\n"
            "0. Выход\n\n"
            "Ваш выбор: ";
        choice = InputChoice();
        if (choice > 5){
            system("cls");
            cout << "Выберите пункт меню >>>\n\n";
        }
    }while (choice > 5);
    return choice;
}
double Gas(data in, const string type[])    // расчет газа
{
    int diffr;
    double g;
    diffr = InputReadout(in, &type[0]);
    g = diffr * price1;
    cin.get();
    cout << "\nИзрасходовано: " << diffr << " м3\n"
        "Сумма оплаты: " << g << "$\n\n"
        "<Enter>";
    cin.get();
    return g;
}
double Light(data in,  const string type[]) // расчет света
{
    int diffr;
    double l;
    diffr = InputReadout(in, &type[1]);
    l = diffr * price2;
    cin.get();
    cout << "\nИзрасходовано: " << diffr << " кВт\n"
        "Сумма оплаты: " << l << "$\n\n"
        "<Enter>";
    cin.get();
    return l;
}
double Water(data in,  const string type[]) // расчет воды
{
    int diffr;
    double w;
    diffr = InputReadout(in, &type[2]);
    w = diffr * price3;
    cin.get();
    cout << "\nИзрасходовано: " << diffr << " м3\n"
        "Сумма оплаты: " << w << "$\n\n"
        "<Enter>";
    cin.get();
    return w;
}
int InputReadout(data in, const string type[])
{
    int diffr;
    do{
        system("cls");
        cout << "Расчет стоимости " << *type << " за " << months[in.month - 1] << "\n\n"
        "Текущее показание: ";
        in.current = InputChoice();
        cout << "Предыдущее показание: ";
        in.previous = InputChoice();
        if (in.current < in.previous){      // проверка - первое показание должно быть > второго
            system("cls");
            cout << "Текущее показание не может быть\n"
                "меньше чем предыдущее!!!\n"
                "Введите корректные показания...\n\n"
                "<Enter>";
            cin.get(); cin.get();
        }
    }while (in.current < in.previous);
    diffr = in.current - in.previous;
    return diffr;
}
int InputChoice()
{
    int choice;
    while (!(cin >> choice) || choice < 0){
        cin.clear();
        while (cin.get() != '\n')
            continue;
        cout << "\nСделайте корректный ввод >>> ";
    }
    return choice;
}
double TotalSum(pabServ tot, data in)
{
    tot.sum = tot.gas + tot.light + tot.water;
    if (tot.sum > 0){
        cout << "Сумма расчетов за " << months[in.month - 1] << endl << endl <<
            "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n"
            << setw(20) << left << "Газ" << "$ " << setw(8) << left << tot.gas << endl
            << setw(20) << left << "Электроэнергия" << "$ " << setw(8) << left << tot.light << endl
            << setw(20) << left << "Вода" << "$ " << setw(8) << left << tot.water << endl
            <<  "------------------------------\n"
            << setw(20) << "Итого" << "$ " << setw(8) << left << tot.sum << endl
            << "\n<Enter>";
        cin.get(); cin.get();
    }
    else{
        cout << "Вы ничего не рассчитали.\n"
            "Повторите пожалуйста...\n\n"
            "<Enter>";
        cin.get(); cin.get();
    }
    return tot.sum;
}

По логике программы в общем неплохо. Только тебя не смущает, что код функций Gas(), Light(), Water() совпадает с точностью до констант? В своём варианте твоей программы я не зря сделал функцию calc_cost()...

Я смотрю, ты про структуры уже прочитал. Скоро до классов доберёшься )) Только ты применяешь структуры не всегда к месту.

В структуре data собраны вместе логически несвязанные значения. Зачем тогда структура?

Смотрим на функции, где используется эта структура:

  1. int AccountMenu(data input) — используется только поле month.
  2. double Gas(data input, const string type[]), double Light(data input, const string type[]), double Water(data input, const string type[]) — используется только для передачи в InputReadout().
  3. int InputReadout(data input, const string type[]) — реально используется только поле month. Вместо остальных полей можно (нужно!) использовать локальные переменные).
  4. double TotalSum(pabServ total, data input) — используется только поле month.
  5. main() — используются поля month и choice. Но choice используется в качестве локальной переменной (это значение вообще больше ни где не используется).

Итого, из всех полей структуры data действительно необходимо только поле month.

Т.е. везде, где в сигнатурах функций написано data <имя переменной>, можно совершенно спокойно написать int <имя переменной>, и использовать эту переменную для передачи номера месяца.

Не понравилась эквилибристика с адресами для передачи строки в ф-цию InputReadout(). Во-первых, нет необходимости передавать глобальный массив констант в качестве параметра в ф-ции Gas() и т.п. Он и так там доступен. Во-вторых, в ф-цию InputReadout() достаточно передать константную ссылку на строку. Т.е. прототип функции может выглядеть так:

int InputReadout(int month_num, const string& type);  // прототип функции для ввода показаний

или ещё проще:

int InputReadout(int month_num, int type);  // прототип функции для ввода показаний

поскольку массив type_calc в InputReadout() тоже доступен. А с позиций перфекционизма можно написать и так:

enum RecourceLabel { GasLabel = 0, LightLabel, WaterLabel };

int InputReadout(int month_num, RecourceLabel type);  // прототип функции для ввода показаний

Естественно, надо внести соответствующие изменения и в ф-ции Gas() и т.п. для корректного вызова InputReadout().

Ещё один эквилибр здесь:

total.sum = TotalSum(total, input);

// в сочетании с

double TotalSum(pabServ tot, data in)
{
    tot.sum = tot.gas + tot.light + tot.water;
    // неважное поскипано ))
    return tot.sum;
}

Что здесь происходит? При вызове ф-ции TotalSum() создаётся копия переменной total (структура полностью копируется), затем в копии изменяем поле sum, затем возвращаем значение только этого поля, что бы присвоить это значение оригиналу.

Упрощаем:

void TotalSum(pabServ *tot, int month_num);  // указатель на структуру (адрес)
// или так
void TotalSum(pabServ& tot, int month_num);  // ссылка на структуру (адрес)

В обоих случаях имеем возможность изменить значение непосредственно в структуре, которая передаётся в качестве параметра.

В качестве затравки:
Структура pabServ просто просится быть преобразованной в класс. Туда же подтянутся price1(2, 3), type_calc и enum RecourceLabel. Суммы по газу, свету, воде можно держать в массиве, также, как и расценки.

Верно, когда я редактировал последний вариант кода, я обратил внимание на функции Gas(), Light(), Water(). Мне так и захотелось собрать их воедино. Только не все так просто... Этот код только пиджак в который осталось впихнуть самое главное расчет газа, света и воды. В каждом из этих расчетов используются свои ограничения по месяцам, в которых определена (или не определена) льгота, соответственно от размера льготы зависит вариант расчета, который будет в корне отличаться от такового за газ, свет или воду (три варианта расчета за газ, три — за свет и два за воду). При этом при выводе каждого и общего расчетов помимо отдельных расчетов и общей суммы, нужно вывести льготу на данные показания, сумму льготы и сумму к оплате (без учета льготы).
То есть, есть ли смысл на данном этапе объединять функции Gas(), Light(), Water(), а потом из них, допустим, вызывать функции расчета? Или вписать код расчетов в нынешние функции?
В общем, согласно твоим корректировкам, код я «причесал»...

#include <iostream>
#include <string>
#include <cstdlib>
#include <iomanip>

using namespace std;

/******************************---------КОНСТАНТЫ ---------*************************************/
const string months[] =
{"январь", "февраль", "март", "апрель", "май", "июнь",
"июль", "август", "сентябрь", "октябрь", "ноябрь", "декабрь"};
const string type_calc[] = {"газа", "электричества", "воды"};
const double price1 = 0.7254;           // цена на газ
const double price2 = 0.2802;           // цена на воду
const double price3 = 3.41;             // цена на свет

/*********************** --------- ПРОТОТИПЫ ФУНКЦИЙ ----------- *******************************/
int MonthMenu();        // прототип функции для выбора месяца
int AccountMenu(int* month);    // прототип функции вывода главного меню
double Gas(int* month);     // прототип функции для вычисления расхода газа
double Light(int* month);   // прототип функции для вычисления расхода света
double Water(int* month);   // прототип функции для вычисления расхода воды
int InputChoice();          // прототип функции проверки на ввод букв
int InputReadout(int* m, int* type);    // прототип функции для ввода паказаний
void TotalSum(int* month, double* total);   // прототип функции для подсчета и вывода общ. результата

int main()
{
    setlocale(LC_ALL, "Russian");

    int month = MonthMenu();
    int choice;
    double total[3] = {0, 0, 0};    // в массиве total 0 - газ; 1 - свет; 2 - вода
    do{
        if (0 == month)
            break;
        system("cls");
        choice = AccountMenu(&month);
        switch(choice){
        case 1 :
            system("cls");
            total[0] = Gas(&month);
            break;
        case 2 :
            system("cls");
            total[1] = Light(&month);
            break;
        case 3 :
            system("cls");
            total[2] = Water(&month);
            break;
        case 4 :
            system("cls");
            TotalSum(&month, total);
            break;              
        case 5 :
// при выборе месяца все расчеты сбрасываются на 0
            system("cls");
            for (int i = 0; i < 3; ++i)
                total[i] = 0;
            month = MonthMenu();
            break;
        }
    }while (choice != 0);
    system("cls");
    cout << "До свидания!\n";
    return 0;
}
int MonthMenu()
{
    int choice;
    do{
        cout << "Выберите месяц:\n\n";
        for (int i = 0; i < 6; ++i){
            cout << setw(2) << right << (i + 1) << ". " << setw(10) << left << months[i]
            << setw(2) << right << (i + 7) << ". " << setw(10) << left << months[i + 6] << endl;
        }
        cout << setw(16) << right << "0. Выход\n\n"
            << "Ваш выбор: ";
        choice = InputChoice();
        if (choice > 12){
            system("cls");
            cout << "Выберите пункт меню >>>\n\n";
        }
    }while (choice > 12);
    return choice;
}
int AccountMenu(int* m)
{
    int choice;
    do{
        cout << "Расчет оплаты коммунальных услуг за " << months[*m - 1] << " месяц\n"
            "\n1. Расчет оплаты за газ\n"
            "2. Расчет оплаты за электричество\n"
            "3. Расчет оплаты за воду\n"
            "4. Общая сумма к оплате за " << months[*m - 1] << "\n"
            "5. Выбор месяца\n"
            "0. Выход\n\n"
            "Ваш выбор: ";
        choice = InputChoice();
        if (choice > 5){
            system("cls");
            cout << "Выберите пункт меню >>>\n\n";
        }
    }while (choice > 5);
    return choice;
}
double Gas(int* m)  // расчет газа
{
    int diffr, type;
    type = 0;
    double g;
    diffr = InputReadout(m, &type);
    g = diffr * price1;
    cin.get();
    cout << "\nИзрасходовано: " << diffr << " м3\n"
        "Сумма оплаты: " << g << "$\n\n"
        "<Enter>";
    cin.get();
    return g;
}
double Light(int* m)    // расчет света
{
    int diffr, type;
    type = 1;
    double l;
    diffr = InputReadout(m, &type);
    l = diffr * price2;
    cin.get();
    cout << "\nИзрасходовано: " << diffr << " кВт\n"
        "Сумма оплаты: " << l << "$\n\n"
        "<Enter>";
    cin.get();
    return l;
}
double Water(int* m)    // расчет воды
{
    int diffr, type;
    type = 2;
    double w;
    diffr = InputReadout(m, &type);
    w = diffr * price3;
    cin.get();
    cout << "\nИзрасходовано: " << diffr << " м3\n"
        "Сумма оплаты: " << w << "$\n\n"
        "<Enter>";
    cin.get();
    return w;
}
int InputReadout(int* m, int* type)
{
    int diffr, current, previous;
    do{
        system("cls");
        cout << "Расчет стоимости " << type_calc[*type] << " за " << months[*m - 1] << "\n\n"
        "Текущее показание: ";
        current = InputChoice();
        cout << "Предыдущее показание: ";
        previous = InputChoice();
        if (current < previous){        // проверка - первое показание должно быть > второго
            system("cls");
            cout << "Текущее показание не может быть\n"
                "меньше чем предыдущее!!!\n"
                "Введите корректные показания...\n\n"
                "<Enter>";
            cin.get(); cin.get();
        }
    }while (current < previous);
    diffr = current - previous;
    return diffr;
}
int InputChoice()
{
    int choice;
    while (!(cin >> choice) || choice < 0){
        cin.clear();
        while (cin.get() != '\n')
            continue;
        cout << "\nСделайте корректный ввод >>> ";
    }
    return choice;
}
void TotalSum(int* m, double* total)
{
    double sum = 0;
    for (int i = 0; i < 3; ++i)
        sum += total[i];
    if (sum > 0){
        cout << "Сумма расчетов за " << months[*m - 1] << endl << endl <<
            "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~\n"
            << setw(20) << left << "Газ" << "$ " << setw(8) << left << total[0] << endl
            << setw(20) << left << "Электроэнергия" << "$ " << setw(8) << left << total[1] << endl
            << setw(20) << left << "Вода" << "$ " << setw(8) << left << total[2] << endl
            <<  "------------------------------\n"
            << setw(20) << "Итого" << "$ " << setw(8) << left << sum << endl
            << "\n<Enter>";
        cin.get(); cin.get();
    }
    else{
        cout << "Вы ничего не рассчитали.\n"
            "Повторите пожалуйста...\n\n"
            "<Enter>";
        cin.get(); cin.get();
    }
}

На данном этапе, предварительно поверхностно ознакомившись с классами, я понимаю, что сейчас весь этот мусор нужно структурировать. Я на пороге знакомства с классами и думаю, что нам будет о чем поговорить :)))
To be continued...

Зачем ты в этих функциях:

int AccountMenu(int* month);    // прототип функции вывода главного меню
double Gas(int* month);     // прототип функции для вычисления расхода газа
double Light(int* month);   // прототип функции для вычисления расхода света
double Water(int* month);   // прототип функции для вычисления расхода воды
int InputChoice();          // прототип функции проверки на ввод букв
int InputReadout(int* m, int* type);    // прототип функции для ввода паказаний
void TotalSum(int* month, double* total);   // прототип функции для подсчета и вывода общ. результата

передаёшь аргументы как указатели? Единственное место, где должен быть указатель — это второй аргумент ф-ции TotalSum(), поскольку это массив. Номер месяца везде лучше передавать просто как int. Указатель в параметрах функции, да ещё неконстантный, всегда наводит на мысль, что этот параметр используется как выходной (т.е. параметр изменяется внутри функции), что, в данном случае, неверно и вводит в заблуждение. Кроме того добавляется лишняя косвенная адресация. Мелочь, но зачем?

Этот код только пиджак в который осталось впихнуть самое главное расчет газа, света и воды. В каждом из этих расчетов используются свои ограничения по месяцам, в которых определена (или не определена) льгота...

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

Thanks a LOT :)
Помимо того, что надо знать, как писать код, нужно понимать как правильно его писать, так, чтобы он работал с минимальными затратами, не был двусмысленным и был понятен для чтения.
Спасибо, Череп, что ты меня поправляешь и направляешь. Я прекрасно понимаю, что все эти «камни в огороде», пойдут только на пользу. Я это вижу даже по тому, что первоначальный код, этой программы, был более чем в 300 строк, а сейчас 200.
Что можно сказать, еще многому нужно научиться и с каждым шагом все интересней и интересней.
СПАСИБО!!! :)))

Внимание! Это довольно старый топик, посты в него не попадут в новые, и их никто не увидит. Пишите пост, если хотите просто дополнить топик, а чтобы задать новый вопрос — начните новый.

Ответить

Вы можете использовать разметку markdown для оформления комментариев и постов. Используйте функцию предпросмотра для проверки корректности разметки.

Пожалуйста, оформляйте исходный код в соответствии с правилами разметки. Для того, чтобы вставить код в комментарий, скопируйте его в текстовое поле ниже, после чего выделите то, что скопировали и нажмите кнопку «код» в панели инструментов. Иначе ваш код может принять нечитаемый вид.

Либо производите оформление кода вручную, следующим образом:

``` #include <iostream> using namespace std; int main() { // ... } ```

Предпросмотр сообщения

Ваше сообщение пусто.