1
\$\begingroup\$

I'm creating a program that finds the roots of Bhaskara’s formula and prints them.

I'm looking for advice to improve my logic more and more.

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

int main ()
{
        double os;
        double four;
        int a;
        int b;
        int c;
        int robf;
        double x1;
        double x2;
        double qisma;
        double jidr;
        //int test;


        os = 2;
        four = 4;
        printf("Enter Number (a) : ");
        scanf("%d",&a);
        printf("Enter number(b) : ");
        scanf("%d",&b);
        printf("Enter number (c) : ");
        scanf("%d",&c);

        qisma = os*a;
        jidr = sqrt((double)(pow(b,os)-(four*a*c)));


        robf = pow(b,os)-(four*a*c);
        x1 = (-b+jidr)/qisma;
        x2 = (-b-jidr)/qisma;
        printf("%d\n",robf);

        if (robf > 0 || robf < 0)
        {
        printf("%s%f\n","Root1 = ",x1);
        printf("%s%f\n","Root2 = ",x2);
        }
        else
        {
                printf("it Imposible to find the roots cuz robf = \n");
        }

return(0);

}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ "Bhaskara's formula" seems to be another name for the quadratic formula. \$\endgroup\$ Nov 3 '21 at 7:56
2
\$\begingroup\$

advice to improve my logic more and more.

Provide link

Rather than assume folks know of Bhaskara’s formula, provide a link, even in a code comment.

Use an auto formatter

Code lacks format consistency, betraying manual formatting. Use an auto formatter for a more professional look. Many IDEs incorporate them.

Check return values

User input is evil. Do not assume valid numeric text was entered.

// scanf("%d",&a);
if (scanf("%d",&a) != 1) Handle_Error_or_other_TBD_code();

Useless cast

(double) serves no purpose in sqrt((double)(pow(b,os)-(four*a*c)));

Consider more direct coding. Use double constants to ensure double math.

sqrt(1.0*b*b - 4.0*a*c);

Define and initialize

// double qisma;
...
// qisma = os*a;
// Instead
double qisma = os*a
 

Don't repeat

Don't repeat, Don't repeat.

    //jidr = sqrt((double)(pow(b,os)-(four*a*c)));
    //robf = pow(b,os)-(four*a*c);
    robf = pow(b,os)-(four*a*c);
    jidr = sqrt(robf);

Guard against sqrt(some negative)

Not later.

    robf = pow(b,os)-(four*a*c);
    if (robf < 0) TBD_Code();
    jidr = sqrt(robf);

Avoid cancellation

Consider the severe cancellation when jidr near |b|. The difference can incur severe lost of precision.

    //x1 = (-b+jidr)/qisma;
    //x2 = (-b-jidr)/qisma;
    
    // Alternative
    if (b < 0) {
      x1 = (-b+jidr)/qisma;
      x2 = c/x1;
    } else if (b > 0) {
      x2 = (-b-jidr)/qisma;
      x1 = c/x2;
    } else {
      x1 = jidr/qisma;
      x2 = -x1;
    }

() not needed

() here is wet.

// return(0);
return 0;

Avoid cancellation 2

In general, 1.0*b*b - 4.0*a*c is prone to cancellation. That inexactness does not apply much here as a,b,c are integers.

When using a,b,c as double, code could use the more stable

double ac4 = 4.0*a*c;
if (ac4 > 0) {
  ac4 = sqrt(ac4);
  robf = (b + ac4)*(b - ac4);
} else {
  robf = b*b - ac4;
}

This is valuable when b*b is about 4.0*a*c.

Useless test

robf < 0 is worthless as prior code would have choked on sqrt((double)(pow(b,os)-(four*a*c))).

Excessive horizontal indenting

IMO, 8 is a bit much. Go with your group's coding standard.

Better prompts

// v  v--spelling           slang-v   v--obscure variable name
//"it Imposible to find the roots cuz robf = \n"
"It is impossible to find the roots because the discriminant is negative\n"

Form a helper function

Rather than code in main(), define a int Bhaskara(double x[2], double a, double b, double c);.

Clean up

What is //int test; in this post for?

\$\endgroup\$
1
\$\begingroup\$

This looks like you have been told never to use magic numbers, and have mistaken that advice:

    double four;

    four = 4;

The only place we use it is in calculating the discriminant (4.0 * a * c), but there it is a necessary part of the formula, not something that could ever be tweaked. If it was really a magic number, then four is the worst possible name for it!

A worse instance of this is the use of os for two completely unrelated quantities that coincidentally happen to have the same value. That's replicating exactly the problem that magic numbers cause - when two constants are the same, we want to know whether or not they have to be identical, or just happen to be so.

Also, we should use const for variables that should not change.

\$\endgroup\$
1
  • \$\begingroup\$ "variables that should not change" always sounds wrong as variables are, well, variable. ;-) Perhaps "for objects that should not change". \$\endgroup\$ Nov 3 '21 at 15:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.