BleepingComputer.com > Here Is A Program I Wrote For Class

ryan_w_quick

Feb 2 2008, 11:44 PM

It just seems really long, repetitve, and clunky for what it does. Can I make it better and more efficient?

-------------------------------------------------------------------------------------------

CODE

//Ryan Quick
//Ohm's Law
#include <stdio.h>
#include <math.h>
#include <time.h>
void stuff ();

void main ()
{
    stuff ();
}

void stuff ()
{
    //Local Declaration

    char unknown;
    double a,b,answer;
    int c,d,e;

    //Statements
    printf("\n\nNote:  Be sure to use standard units for all calculations.\n");
    printf("When asked yes or no questions, answer 1 for yes and 0 for no.\n");
    printf("What do you want to solve for?\n v = voltage \n i = current \n r = resistance \n p = power\n");
    scanf("%c", &unknown);//What you want to solve for?

    //Solving for Voltage

    if(unknown == 'v')
    {
        printf("Do you know current?\n");
        scanf("%d", &c);
            if(c == 1)
            {
                printf("Do you know resistance?\n");
                scanf("%d", &d);
                if(d == 1)
                {
                    printf("What is the current?\n");
                    scanf("%lf", &a);
                    printf("What is the resistance?\n");
                    scanf("%lf", &b);
                    answer = a * b;
                    printf("%lf Volts\n", answer);
                }
                if(d == 0)
                {
                    printf("What is the current?\n");
                    scanf("%lf", &a);
                    printf("What is the power?\n");
                    scanf("%lf", &b);
                    answer = b/a;
                    printf("%lf Volts\n", answer);
                }
            }
            if(c == 0)
            {
                printf("What is the resistance?\n");
                scanf("%lf", &a);
                printf("What is the power?\n");
                scanf("%lf", &b);
                answer = sqrt(a * B);
                printf("%lf Volts\n", answer);
            }
    }

    //Solving for Current

    if(unknown == 'i')
    {
        printf("Do you know voltage?\n");
        scanf("%d", &c);
            if(c == 1)
            {
                printf("Do you know resistance?\n");
                scanf("%d", &d);
                if(d == 1)
                {
                    printf("What is the voltage?\n");
                    scanf("%lf", &a);
                    printf("What is the resistance?\n");
                    scanf("%lf", &b);
                    answer = a / b;
                    printf("%lf Amperes\n", answer);
                }
                if(d == 0)
                {
                    printf("What is the voltage?\n");
                    scanf("%lf", &a);
                    printf("What is the power?\n");
                    scanf("%lf", &b);
                    answer = b/a;
                    printf("%lf Amperes\n", answer);
                }
            }
            if(c == 0)
            {
                printf("What is the resistance?\n");
                scanf("%lf", &a);
                printf("What is the power?\n");
                scanf("%lf", &b);
                answer = sqrt(b/a);
                printf("%lf Amperes\n", answer);
            }
    }

    //Solving for Resistance

    if(unknown == 'r')
    {
        printf("Do you know voltage?\n");
        scanf("%d", &c);
            if(c == 1)
            {
                printf("Do you know current?\n");
                scanf("%d", &d);
                if(d == 1)
                {
                    printf("What is the voltage?\n");
                    scanf("%lf", &a);
                    printf("What is the current?\n");
                    scanf("%lf", &b);
                    answer = a / b;
                    printf("%lf Ohms\n", answer);
                }
                if(d == 0)
                {
                    printf("What is the voltage?\n");
                    scanf("%lf", &a);
                    printf("What is the power?\n");
                    scanf("%lf", &b);
                    answer = a * a / b;
                    printf("%lf Ohms\n", answer);
                }
            }
            if(c == 0)
            {
                printf("What is the current?\n");
                scanf("%lf", &a);
                printf("What is the power?\n");
                scanf("%lf", &b);
                answer = (B) / (a*a);
                printf("%lf Ohms\n", answer);
            }
    }

    //Solving for Power

    if(unknown == 'p')
    {
        printf("Do you know voltage?\n");
        scanf("%d", &c);
            if(c == 1)
            {
                printf("Do you know current?\n");
                scanf("%d", &d);
                if(d == 1)
                {
                    printf("What is the voltage?\n");
                    scanf("%lf", &a);
                    printf("What is the current?\n");
                    scanf("%lf", &b);
                    answer = a * b;
                    printf("%lf Watts\n", answer);
                }
                if(d == 0)
                {
                    printf("What is the voltage?\n");
                    scanf("%lf", &a);
                    printf("What is the resistance?\n");
                    scanf("%lf", &b);
                    answer = a * a / b;
                    printf("%lf Watts\n", answer);
                }
            }
            if(c == 0)
            {
                printf("What is the current?\n");
                scanf("%lf", &a);
                printf("What is the resistance?\n");
                scanf("%lf", &b);
                answer = a * a * b;
                printf("%lf Watts\n", answer);
            }
    }
    main ();
}

ryan_w_quick

Feb 2 2008, 11:46 PM

i don't know why it did those smiley, but they should be a b followed by a parentheses

b )

groovicus

Feb 3 2008, 08:53 AM

I fixed your post to use the code tags so that the code would format properly, otherwise it is too hard to read.

Anytime you are repeating code, it becomes a candidate for writing a reusable function. You also have some statements that can be combined. For instance,

CODE

answer = a * b;
printf("%lf Volts\n", answer);

Could be combined into:

CODE

printf("%lf Volts\n", (a*b));

That in itself would eliminate a dozen lines of code.

You use the following code 6 times:

CODE

printf("What is the current?\n");
scanf("%lf", &a);

That makes it a candidate for writing a function. Something like this:

CODE

int getCurrent(){
    printf("What is the current?\n");
    scanf("%lf", &a);
    return a;
}

Now when you need to get the current, you use something like:

CODE

a= getCurrent();

I have not coded in any c-based language for awhile, so my syntax is probably not correct. You can do the same thing with getResistance, getPower, etc. That would clear up a bunch of your code.

You don't need to worry about efficiency. If your code was 100000 lines long, then maybe.

One final comment on your variable names. Unless they are being instantiated and used right away in just one spot, they should never be called a,b,c, etc. They should have useful names that lets you know exactly what that variable is holding. A variable named current allows me to better understand the code. A varaible named unknown is equally useless. You might know what it means, but someone else trying to look at your code won't have a clue.

ryan_w_quick

Feb 3 2008, 02:33 PM

yeah my prof gets on me for those short meaningless variable names. I guess I'll fix that, along with putting in those functions. Thanx for the tips, and yeah that syntax is mostly right, thanx.

rongchaua

Feb 3 2008, 05:48 PM

@ryan: I'm not professional but I have some tips. Maybe they are good for you.

1. Use if...else... or switch case... instead of only if. Try to look at your structure.

if(unknown == 'v')...
if(unknown == 'i')...
if(unknown == 'r')....

The program will be slowed down because it must always test all of cases. For example, I choose 'v', and the program calculates, displays the results.... and after solving the case (unknown =='v') it will be test straighforward "if (unknown=='i'), (unknown=='r')..." it is senseless. Because unknown can not be i,r,p anymore. And the other programmers who can resume later your work, will have difficulty to recognize the flow control of program and the relation between cases.

2. Try to catch the error as well as you can. What will it occur when I do not enter one of v,i,r,p. The main() function will also be called. Only God knows what will happen. And there are also more bugs in your program about catching exception.

3. answer = a * a / b; <-- Be carefull with the priority of operation. Use bracket to make it clearer. It's just better for you. Because sometimes it'll make you crazy to figure out where the error is.

Hope you will not be annoyed with my tips. I do not intend to "teach" you. Just share you some tips which I use in my work.

ryan_w_quick

Feb 3 2008, 10:34 PM

I know i need to work more on the sensless and repetitive code, but I'm not to woried about the math and order of operations and precedence that you are talking about. I usualy use a lot of parentheses to keep things very clear in my mind, but this is a formula i'm very familiar with and just didnt feel the need.