Пожалуйста откритикуйте код на си и дайте советов?

Question

[Kaktys_DH]

#include <stdio.h>
int main()
{

float num_1 = 0;
float num_2 = 0;
float result = 0;
char wait[2] = {0};
char vopros[2] = {0};
char vopros_2[2] = {0};

do {
printf("\n\t\t\t\t\tКАЛЬКУЛЯТОР 3.0");

printf("\nВведите первое число: ");
scanf("%f", &num_1);

printf("\nВведите второе число: ");
scanf("%f", &num_2);

printf("\nВыберите действие - +, -, *, /: ");
scanf("%s", wait);

if (wait[0] == '+') {
result = num_1 + num_2;
}

else if (wait[0] == '-') {
result = num_1 - num_2;
}

else if (wait[0] == '*') {
result = num_1 * num_2;
}

else if (wait[0] == '/') {
result = num_1 / num_2;
}

else {
  printf("Ошибка синтаксиса\n");
}

if (  ( wait[0] == '/' && (num_1 == 0 && num_2 == 0) )  ||  ( wait[0] == '/' && (num_1 == 0 || num_2 == 0) )  ) {

printf("\n\nТЫ ЧТО-ТО ДЕЛАЕШЬ НЕ ТАК!!!\n\n");
}
goto metka;

printf("%f %s %f = %f\n", num_1, wait, num_2, result);

do {
printf("Хотите воспроизвести действие еще с одним числом?(Y/n)");
scanf("%s", vopros_2);

if (vopros_2[0] == 'n') {
   vopros_2[0] = 'N';
}

else if (vopros_2[0] != 'n' || vopros_2[0] != 'N') {
num_1 = 0;
num_2 = 0;
char wait[2] = {0};

printf("Введите число\n");
scanf("%f", &num_1);

printf("\nВыберите действие - +, -, *, /: ");
scanf("%s", wait);

if (wait[0] == '+') {
result = result + num_1;
}
else if (wait[0] == '-') {
result = result - num_1;
}
else if (wait[0] == '*') {
result = result * num_1;
}
else if (wait[0] == '/') {
result = result / num_1;
}
else {
  printf("Ошибка синтаксиса\n");
}

printf("Result - %.6f\n", result);
}


}
while (vopros_2[0] != 'N');
metka:
printf("Хотите попробывать еше раз? (Y/n): ");
scanf("%s", vopros);

if (vopros[0] == 'n') {
vopros[0] = 'N';
}

}
while (vopros[0] != 'N');

return 0;
}

Comments

[Александр]

Советую вставлять код через специальный тег, код без тега мало кто читать будет.

[Василий Дёмин]

Проверку деления на ноль нужно вставлять перед самим делением. Также она у вас неверно сделана, нужно проверять на равенство нулю только num_2. Также не понятно, зачем нужен код между goto metka и самой меткой metka, он никогда не выполнится, нужно убрать его вместе с goto и меткой.

[Kaktys_DH]

Александр, как?

[Kaktys_DH]

Василий Дёмин, это нужно что бы пропустить тот участок кода если пользователь введёт 0

[Василий Дёмин]

Kaktys_DH, Переход к метке происходит независимо от введённых чисел. Можно сделать проще, например, завести переменную correct, значение которой будет устанавливаться в false, если пользователь ввёл ноль. В случае, если correct равна нулю printf не выводит ответ.

Я бы переделал код таким образом:

#include <stdio.h>
#include <stdbool.h>

int main() {
    float num_1, num_2, result;
    char wait, vopros;
    bool correct;

    printf("\n\t\t\t\t\tКАЛЬКУЛЯТОР 3.0");
    do {
        correct = true;
        printf("\nВведите первое число: ");
        scanf("%f", &num_1);

        printf("\nВведите второе число: ");
        scanf("%f", &num_2);

        printf("\nВыберите действие +, -, *, /: ");
        scanf("%01s", &wait);

        switch(wait) {
            case '+':
                result = num_1 + num_2;
                break;
            case '-':
                result = num_1 - num_2;
                break;
            case '*':
                result = num_1 * num_2;
                break;
            case '/':
                if(num_2 == 0) {
                    printf("\n\nТЫ ЧТО-ТО ДЕЛАЕШЬ НЕ ТАК!!!\n\n");
                    correct = false;
                } else {
                    result = num_1 / num_2;
                }
                break;
            default:
                printf("Ошибка синтаксиса\n");
                correct = false;
        }

        if(correct)
            printf("%f %c %f = %f\n", num_1, wait, num_2, result);
        printf("Хотите попробовать еше раз? (Y/n): ");
        scanf("%01s", &vopros);
    } while(vopros != 'n' && vopros != 'N');
}

[CityCat4]

Хм, вижу некоторый но не слишком великий прогресс :)

- проверку на нуль нужно делать ДО момента деления, потому что программа вылетит с SIGILL в момент выполения деления на нуль.
- использовать goto - признак говнокода. Старайся держаться от goto как можно дальше.
- switch вместо какскадного if
- toupper/tolower

Сравни-ка свой код и код от Василий Дёмин :)

Answer 1

[AnT]

Проблемы, в порядке возникновения

1. В С предпочитайте int main(void) вместо int main()
2. Необоснованное использование типа float вместо типа double.
3. Какая-то странная смесь нелокализованных и локализованных объявлений переменных, в т.ч. переменных с одинаковыми именами в вложенных областях видимости. Напр. wait
4. scanf("%s", wait); никак не защищен от переполнения. И это при том, что буфер wait имеет минимальный размер. Лучше scanf("%1s", wait);. Разумеется, это не решит всех потенциальных проблем с некорректным вводом...
5.

if (  ( wait[0] == '/' && (num_1 == 0 && num_2 == 0) )  ||  ( wait[0] == '/' && (num_1 == 0 || num_2 == 0) )  )

- что это за странная двойная проверка одного и того же?
6. goto здесь совсем не уместно.
7. Что это вообще за странное дублирование одного и тот же кода?

Comments

[Kaktys_DH]

1. Ок
2. Забыл в тот момент про дабл, спасибо что напомнил
4. Сделаю защиту
5. А что если пользователь введёт 0 в num_1, будет ошибка, а это оберегает.
6. Не нашел другого
7. Я просто дополнил