HiveBrain v1.2.0
Get Started
← Back to all entries
patterncModerate

Parabola calculations

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
calculationsparabolastackoverflow

Problem

I'm trying to make a small parabola calculator just for fun. Is this code good?

#include 
#include 
#include 
#include 

#define _X_UNTIL_Y(q,w)          \
    while (argv[1][q] != w) {    \
        q++;                     \
    }

#define _ATOI(s,d,f,g)           \
    strncpy(s, argv[1]+f, g);    \
    d = atoi(s)

int main(int argc, char const *argv[]) {
    int i=0, j, l;
    float D, a=1, b=1, c=1, r, k;
    char A[5], B[5], C[5], d;

    if(argv[1][0] == 'x');
    else{
        _X_UNTIL_Y(i,'x');
        _ATOI(A,a,0,i);
    }
    if(argv[1][0] == '-' && argv[1][1] == 'x') a=-1;

    j = i += 3;
    _X_UNTIL_Y(j,'x');
    if(argv[1][j-1] == '+' || argv[1][j-1] == '-');
    else{
        _ATOI(B,b,i,j);
    }
    if(argv[1][j-1] == '-') b=-b;

    l = j += 1;
    _X_UNTIL_Y(l,'=');
    _ATOI(C,c,j,l);

    D = pow(b,2)-4*a*c;

    if(D  0){
        d = 'p';
        printf("Parabola has two real roots.\n");
    }else
        return -1;

    r = -(b/(2*a));
    k = -(D/(4*a));

    a < 0 ? printf("Parabola look downwards.\n") : printf("Parabola look upwards.\n");
    printf("Vertex: %.1f,%.1f\n", r, k);
    printf("Axis of symmtery: %.1f\n", r);
    printf("y-intercept: %.1f\n", c);

    if(d=='n');
    else if(d=='s')
        printf("x-intercept: %.1f\n", r);
    else if(d=='p'){
        float x1, x2;
        x1=(-b + sqrt(D)) / (2*a);
        x2=(-b - sqrt(D)) / (2*a);
        printf("x-intercepts: %.1f, %.1f\n", x1, x2);
    }
    return 0;
}

Solution

I see a number of things that may help you improve your code.

Avoid macros

The macros _X_UNTIL_Y and _ATOI do not help with readability. I'd advise not using macros like those, preferring functions instead.

Use more descriptive variable names

All of the single-letter variable names make it difficult to decipher the algorithm. More meaningful variable names make the code easier to read and maintain.

Break up the code into smaller functions

Rather than having everything in one long main function, it would be easier to read and maintain if each discrete step were its own function.

Think of the user

If the user runs the code with no arguments, he or she is rewarded with a segfault and crash. It would be much nicer instead to prompt the user with a string describing how the program is intended to be used instead.

Use idiomatic C

Lines like these:

if(argv[1][0] == 'x');
else{


are quite odd. It would make it much easier to read and understand if it were written instead like this:

if(argv[1][0] != 'x') {


Add comments

Code like this is difficult enough to read for the reasons listed above. Just a few well-placed comments about why the code is doing what it's doing would greatly aid readers of the code to understand your intent.

Eliminate return 0 at the end of main

Since C99, the compiler automatically generates the code corresponding to return 0 at the end of main so there is no need to explicitly write it.

Code Snippets

if(argv[1][0] == 'x');
else{
if(argv[1][0] != 'x') {

Context

StackExchange Code Review Q#127157, answer score: 10

Revisions (0)

No revisions yet.