C программирование хорошая практика или нет?

Question

Я написал небольшой кусок кода для вычисления квадратичных уравнений, но если дискриминант отрицательный, я хотел, чтобы он написал, что для этого квадратичного уравнения нет реальных численных значений. Чтобы это произошло, мне пришлось вызвать функцию с четвертым параметром 0, который, я думаю, я не знаю, почему, было бы плохой практикой программирования? Это так, или я просто слишком разборчив с моим кодом? Спасибо. (Причина, по которой я спрашиваю об этом, заключается в том, что я не хочу забирать некоторые вредные привычки на ранней стадии моей карьеры программирования). Вот код.

#include <stdio.h> 
#include <math.h> 
#include <string.h> 

double quadratic_equation(double a, double b, double c, double d); 

int main(void) 
{ 
    char command[20]; 
    int i; 

    printf("Enter your command: "); 
    fgets(command, 20, stdin); 

    for (i = 0; i < 20; i++) { 
     if (command[i] == '\n') { 
      command[i] = '\0'; 
      break; 
     } 
    } 

    if (strcmp(command, "quadratic equation") == 0) { 
     double a, b, c, x; 

     printf("Enter A: "); 
     scanf("%lf", &a); 
     printf("Enter B: "); 
     scanf("%lf", &b); 
     printf("Enter C: "); 
     scanf("%lf", &c); 

     x = quadratic_equation(a, b, c, 0); // THIS PIECE HERE MIGHT BE BAD PRACITCE ? 

     if (x == 0) { 
      printf("There are no real numerical values to this quadratic equation."); 
     } 

     else { 
      printf("------------\n"); 
      printf("x1 = %.2f\n", quadratic_equation(a, b, c, 1)); 
      printf("x2 = %.2f", quadratic_equation(a, b, c, -1)); 
     } 
    } 

    return 0; 
} 

double quadratic_equation(double a, double b, double c, double d) { 
    double discriminant, x, insideroot; 

    insideroot = ((b*b) - (4*a*c)); 

    if (insideroot < 0) { 
     return 0; 
    } 

    discriminant = sqrt(insideroot); 
    x = (-b + (d * discriminant))/(2 * a); 

    return x; 
} 

Большое спасибо за помощь: d!

Answer 1

Я бы сказал, что это не здорово. Что вы могли бы сделать что-то вроде этого:

int quadratic_equation(double a, double b, double c, double *root_a, double *root_b) { 
    double discriminant = ((b*b) - (4*a*c)); 

    if (discriminant < 0) { 
     return -1; 
    } 

    if (root_a != NULL) { 
     *root_a = (-b + sqrt(discriminant))/(2 * a); 
    } 
    if (root_b != NULL) { 
     *root_b = (-b - sqrt(discriminant))/(2 * a); 
    } 

    return 0; 
} 

Тогда вы могли бы назвать это так:

double root_a; 
double root_b; 
int ok = quadratic_equation(a, b, c, &root_a, &root_b); 
if (ok < 0) { 
    // It wasn't OK. Print out an error. 
} else { 
    // It was OK. Print out the results. 
} 

Обратите внимание, что вы должны также проверить другие случаи ошибок в функции и возвращать -1 для них Что ж. Например. a - ноль.

Answer 2

Это, безусловно, плохая практика. Так как корни формулы a, b и c a be any double вы do нужно какое-то прохождение.

Я бы предложил параметр, являющийся указателем на int. Если указатель NULL игнорируется, в противном случае он будет установлен в 1 или 0 в зависимости от того, существует ли реальный корень:

double quadratic_equation(double a, double b, double c, int *root_exists) { 
    double discriminant; 

    discriminant = ((b*b) - (4*a*c)); 

    if (discriminant < 0) { 
     if (root_exists != NULL) *root_exists = 0; 
     return 0.0; 
    } 

    x = (-b + sqrt(discriminant))/(2 * a); 

    if (root_exists != NULL) *root_exists = 1; 

    return x; 
} 

Более строгий подход заключается в следующем:

typedef struct { 
    int num_roots; 
    double roots[2]; 
} quadratic_roots_t; 

quadratic_roots_t quadratic_equation(double a, double b, double c) { 
    quadratic_roots_t roots; 
    double d; 

    d = b*b - 4*a*c; 

    if (d < 0.0) { 
     roots.num_roots = 0; 
    } else if (d == 0.0) { 
     roots.num_roots = 1; 
     roots.roots[0] = -b/(2 * a); 
    } else { 
     roots.num_roots = 2; 
     roots.roots[0] = (-b - sqrt(d))/(2 * a); 
     roots.roots[1] = (-b + sqrt(d))/(2 * a); 
    } 

    return roots; 
} 

Answer 3

Я бы, конечно, позвонить это плохая практика, потому что код очень неясен.

Во-первых, вы вызываете функцию три раза, когда когда-то должно быть достаточно.

Я бы подумал о возврате/заполнении списка в вашей функции quadratic_equation() вместо того, чтобы возвращать корни один за другим.

Это также позволит вам определить, существуют ли реальные корни - если их нет, просто верните пустой список.

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

Answer 4

Рассмотрите возможность использования возвращаемого значения, чтобы указать, работает ли все, и передача массива в функцию, чтобы получить возвращаемые значения:

enum QE_Status { QE_OK = 0, QE_NON_QUADRATIC, QE_COMPLEX_ROOTS, QE_NULL_POINTER }; 

enum QE_Status quadratic_equation(double a, double b, double c, double *r) 
{ 
    double discriminant; 

    if (r == 0) 
     return QE_NULL_POINTER; 
    if (a == 0.0) 
     return QE_NON_QUADRATIC; 

    discriminant = (b * b) - (4 * a * c); 

    if (discriminant < 0) 
     return QE_COMPLEX_ROOTS; 

    discriminant = sqrt(discriminant); 
    r[0] = (-b + discriminant)/(2 * a); 
    r[1] = (-b - discriminant)/(2 * a); 
    return QE_OK; 
} 

Вы можете расширить систему для обработки численной неустойчивости (поскольку b*b почти равна до 4*a*c, или потому, что a очень маленький и т. д.).

Вызывающий код затем может быть:

double a, b, c, x[2]; 

if (quadratic_equation(a, b, c, x)) 
    ...oops, something went wrong... 

Или:

switch (quadratic_equation(a, b, c, x)) 
{ 
case QE_OK: 
    ...print or use results in x... 
    break; 
case QE_NON_QUADRATIC: 
case QE_COMPLEX_ROOTS: 
    ...print appropriate error message about user's data... 
    break; 
case QE_NULL_POINTER: 
    ...Oops - programming error... 
    break; 
}