Тестирование программы, которая вычисляет дни, оставшиеся в месяце

Я новичок в кодировании C ++, и я хочу получить представление о моем прогрессе. Можете ли вы, ребята, посмотреть мою программу и дать мне несколько отзывов об этом?

#include <iostream>
using namespace std;

int main(){

int day{};
int monthNum{};
cout << "Calculate how many days are left in the month. \nPlease enter the month number and the day, seperated by a space. ex 1 22 for Jan 22nd: ";
cin >> monthNum >> day;

if (monthNum < 1 || monthNum > 12 || day < 1 || day > 31 || (monthNum == 2 && (day < 1 || day > 29))){
    cout << "Error, not a valid input. \nPlease make sure you're month and day are valid." << endl;
    return 0;
}

int daysInCurrentMonth{};
char leapYear;
int doNothing{};  //Used to do nothing if certain conditions are false

if (monthNum == 2){
    cout << "\nIs it leap year? Type y for yes and n for no. ";
    cin >> leapYear;
    ((leapYear == 'y') ? daysInCurrentMonth = 29 : daysInCurrentMonth = 28);
    if (leapYear != 'y' && leapYear != 'n'){
        cout << "invalid input..." << endl << endl;
        return 0;
    }else{ 
        doNothing = 0;
    }
}

cout << "\nYou entered day " << day << " of month " << monthNum << endl;

((monthNum == 4 || monthNum == 6 || monthNum == 9 || monthNum == 11) ? daysInCurrentMonth = 30 : doNothing = 0);
((monthNum == 1 || monthNum == 3 || monthNum == 5 || monthNum == 7 || monthNum == 8 || monthNum == 10 || monthNum == 12) ? daysInCurrentMonth = 31 : doNothing = 0);

int daysLeftInMonth{daysInCurrentMonth - day};
cout << "\nThere are " << daysLeftInMonth << " days left in the month." << endl;

cout << endl;
return 0;
}

Это моя третья итерация этой программы. Это - это серия учебников, я смотрел. В настоящее время я нахожусь в разделе 9 и только что закончил изучение условных операторов. Я пытаюсь уточнить проект, когда я иду. Кроме того, я знаю, что довольно рано думать об этом, но если кто-то может это сделать, я хотел бы знать, правильно ли он структурирует это, чтобы стать профессиональным программистом, то есть легко следовать и читаемый, или он выглядит как мусор.

8 голосов | спросил Thomas Margraff 23 J0000006Europe/Moscow 2018, 16:20:12

2 ответа


5
  • Не используйте using namespace std

  • Предпочитаете использовать \n over std::endl

  • Последний return 0 не нужен в этом случае, поскольку компилятор будет генерировать его для вас

  • При использовании тернарного оператора вы можете просто назначить в начале вместо того, чтобы делать это в обеих ветвях

  • Не часть кода, но вы должны проверить правописание

Я не уверен, что вы делаете ввод с целью узнать, как это работает, но это можно сделать без взаимодействия с пользователем.
Посмотрите ctime , так как он предлагает множество функций, которые могут быть полезны здесь.

Вот небольшая программа, демонстрирующая, как использовать эти функции:

#include <ctime>
#include <iostream>

int main() {
    std::time_t time = std::time(nullptr);
    auto today = std::localtime(&time);

    int day = today->tm_mday;
    int month = today->tm_mon + 1;
    int year = today->tm_year + 1900;
    int month_length = 30;

    constexpr int february = 2;
    if (month == february) {
        month_length = is_leapyear(year) ? 29 : 28;
    }

    std::cout << month_length - day << " days remain in this month.\n";
}

Так как функция is_leapyear тривиальна, я ее оставил.

Конечно, если вы используете C ++ 20, вы можете просто использовать chrono , который предлагает удивительное количество функций, связанных с временем и датой.

ответил yuri 23 J0000006Europe/Moscow 2018, 18:01:25
5

Как Юрий уже сказал в своем ответе , не используйте using namespace std. Я также рекомендую вам прочитать на этом сайте некоторые из Q & A в теге , вы узнаете кучу подобных ошибок начинающих.

Ваши имена переменных велики, ясно, что их цель не требует добавления комментариев.

doNothing не имеет цели. Вы устанавливаете его в разных местах, но вы его нигде не читаете. Однако, если вы его использовали, я бы рекомендовал вам объявить его как bool (который может иметь значения true или false). Наконец, вы инициализируете его как int doNothing{};, который дает ему значение 0 и везде, где вы присваиваете ему значения, вы назначаете 0 тоже, поэтому эти присваивания ничего не делают.

Этот бит кода:

if (monthNum == 2){
    //...
}
((monthNum == 4 || monthNum == 6 || monthNum == 9 || monthNum == 11) ? daysInCurrentMonth = 30 : doNothing = 0);
((monthNum == 1 || monthNum == 3 || monthNum == 5 || monthNum == 7 || monthNum == 8 || monthNum == 10 || monthNum == 12) ? daysInCurrentMonth = 31 : doNothing = 0);

имеет много избыточности. Вы проверяете на месяц 2, затем на 30-дневные месяцы, затем на оставшиеся 7 месяцев. Вы можете упростить это как:

if (monthNum == 2) {
    // 28/29 days
} else if (monthNum == 4 || monthNum == 6 || monthNum == 9 || monthNum == 11) {
    // 30 days
} else {
    // 31 days
}

Но было бы даже более аккуратно настроить таблицу поиска:

int daysInMonth[] = {0,31,28,31,30,31,30,31,31,30,31,30,31}; // element[0] not used
int daysInCurrentMonth = daysInMonth[monthNum];
// ...and then check for leap years

Большая часть логики в main может быть отделена от функций. Чем короче каждая функция, тем легче проверить ее правильность, и тем легче ее читать.

Объявлять переменные в области, в которой вы их используете:

char leapYear;
if (monthNum == 2){
    cout << "\nIs it leap year? Type y for yes and n for no. ";
    cin >> leapYear;
    //...
}
//... leapYear is never used from here on

Вы должны вместо этого объявить leapYear внутри области if, поэтому он не будет существовать вне этой области. Чем короче область переменных, тем легче читать программу и тем меньше ошибок, которые вы сделаете.

При выходе из программы с ошибкой обычно возвращается ненулевое значение (по крайней мере, на POSIX-системах, я не знаю, делает ли Windows что-либо с возвращаемым значением?)

std::cout << "invalid input...\n\n";
return 1;

Наконец, я лично считаю это запутанным:

int day{};

Инициализирует day до 0, но делает это неочевидным способом. Возможно, это более личное предпочтение, чем что-либо другое, но я бы предложил

int day = 0;

, если вам нужно инициализировать day или просто

int day;

, так как вы будете писать в него сразу после и никогда не будете читать значение, которое было инициализировано. Ваш курс, вероятно, стучит по единой инициализации. Наверное, я старомодный, думая, что оператор = хорош и ясен. :)

ответил Cris Luengo 23 J0000006Europe/Moscow 2018, 20:09:47

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132