Look at my crappy program. Look at it!

This is a discussion on Look at my crappy program. Look at it! within the C Programming forums, part of the General Programming Boards category; So, I managed to get up to the Functions section of the C tutorials. With my newly gained knowledge, I ...

  1. #1
    Registered User
    Join Date
    Mar 2009
    Posts
    7

    Angry Look at my crappy program. Look at it!

    So, I managed to get up to the Functions section of the C tutorials. With my newly gained knowledge, I made a fricking calculator. A pretty crappy calculator, I'm not afraid to say, but the first program I ever made by myself. So . . . look at it! Appraise it and give me criticism. Please? >.>

    Code:
    #include <stdio.h>
    
    int add (int A, int S);
    int dif (int D, int F);
    int mult (int Q, int W);
    int div (int E, int R);
    
    int main()
    {
        int A,S,D,F,Q,W,E,R;
        int sheep=1,skin=2,seat=3,covers=4;
        int dumbass;
    
        /* I know I have weird variable names that don't pertain, at all, to their function.
        That was kind of the point. If you have to look at my crappy code, you may as well have a reason to smile about it.
        Does that compute? */
    
        printf( "This is Hiru's crappy calculator.\nPlease choose which crappy operation you would like to perform:\n" );
        printf( "(1) Addition; (2) Subtraction; (3) Multiplication; (4) Division: " );
        scanf( "%d", &dumbass );
            if (dumbass==sheep){
                printf( "Enter the two numbers to be added:\n" );
                scanf( "%d", &A );
                scanf( "%d", &S );
                printf( "Your numbers were %d and %d. Their sum is: %d\n", A, S, add (A,S) );
                getchar();
            }
            else if (dumbass==skin){
                printf( "Enter the two numbers to be subtracted:\n" );
                scanf( "%d", &D );
                scanf( "%d", &F );
                printf( "Your numbers were %d and %d. Their difference is: %d\n", D, F, dif (D,F) );
                getchar();
            }
            else if (dumbass==seat){
                printf( "Enter the two numbers to be multiplied:\n" );
                scanf( "%d", &Q );
                scanf( "%d", &W );
                printf( "Your numbers were %d and %d. Their product is: %d\n", Q, W, mult (Q,W) );
                getchar();
            }
            else if (dumbass==covers){
                printf( "Enter the two number to be divided:\n" );
                scanf( "%d", &E );
                scanf( "%d", &R );
                if (R == 0){
                printf( "I'm sorry, Dave. I'm afraid I can't do that.\n" );}
                else if (E == 0){
                printf( "Uh, you can't divide nothing.\nNo matter how badly you want to share it with your friends.\n" );}
                else printf( "Your numbers were %d and %d. Their quotient is: %d\n", E, R, div (E,R) );
                getchar();
            }
    }
    
    int add (int A, int S)
    {
        return A + S;
    }
    int dif (int D, int F)
    {
        return D - F;
    }
    int mult (int Q, int W)
    {
        return Q * W;
    }
    int div (int E, int R)
    {
        return E / R;
    }
    -Hiru
    Last edited by Hirumaru; 03-11-2009 at 06:42 AM.

  2. #2
    Kernel hacker
    Join Date
    Jul 2007
    Location
    Farncombe, Surrey, England
    Posts
    15,677
    Here's some suggestions:
    - No offensive words in variables (dumbass).
    - Constants that MEAN something make the code easier to read. If they are just names instead of numbers, it doesn't make any sense.
    - Single letter variable names (in upper-case, no less) is best avoided.
    - two lines of scanf() to read two numbers can be turned into a single scanf("%d %d", &a, &b);
    - Dividing zero with non-zero is valid, so there's no reason to check for that.

    --
    Mats
    Compilers can produce warnings - make the compiler programmers happy: Use them!
    Please don't PM me for help - and no, I don't do help over instant messengers.

  3. #3
    Woof, woof! zacs7's Avatar
    Join Date
    Mar 2007
    Location
    Australia
    Posts
    3,459
    1. C89 at least defines only 2 variations of main,
    int main(void)
    int main(int argc, char * argv[])

    Main also must return something (explicitly is best).

    Pick one on a case-by-case basis, note that int foo() is NOT the same as int foo(void).

    2. The variable names are terrible, they don't even relate to what the program does
    3. The brace style and indenting is inconsistant
    4. There is very little error checking offered with scanf(). Such as overflow, consider using fgets() and strtol() or similar.

    Is it some sort of joke that it reads "dumbass sheep skin seat covers"? I have them in my car, very comfortable
    Last edited by zacs7; 03-11-2009 at 06:27 AM.

  4. #4
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,588
    Quote Originally Posted by zacs7
    Pick one on a case-by-case basis, note that int foo() is NOT the same as int foo(void).
    In this case int foo() is the same as int foo(void) because it is part of a function definition.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  5. #5
    Registered User
    Join Date
    Mar 2009
    Posts
    7
    Quote Originally Posted by matsp View Post
    Here's some suggestions:
    - No offensive words in variables (dumbass).
    Sowwy. :P

    - Constants that MEAN something make the code easier to read. If they are just names instead of numbers, it doesn't make any sense.
    - Single letter variable names (in upper-case, no less) is best avoided.
    - two lines of scanf() to read two numbers can be turned into a single scanf("%d %d", &a, &b);
    - Dividing zero with non-zero is valid, so there's no reason to check for that.

    --
    Mats
    - Just giving it a personal touch. ^___^
    - I know what you mean, but a single letter variable should be okay for a five-minute program. :P
    - Awesome. I suspected as much, but didn't quite figure that that was how it was written.
    - I know, but still. Zero out of anything is still zero. Why the hell would you want to divide nothing? You already know the answer just by looking at it. XD

    EDIT:
    @Zacs
    1. I'm at lesson 4, not lesson 104. =.=
    2. Precisely!
    3. Only because this is a small program, with superfluous things. :P
    4. As soon as I learn what the heck those are . . .

    Is it some sort of joke that it reads "dumbass sheep skin seat covers"? I have them in my car, very comfortable
    Heh. "Sheepskin seat covers" is reference to "Ed, Edd, and Eddy". "Dumbass" comes out of my self-abusive nature. I kept forgetting semicolons and making newbie mistakes, so I made the text responses address me as "dumbass". "Declare integers first, dumbass." "You forgot a semicolon again, dumbass." Et cetera.

    -Hiru
    Last edited by Hirumaru; 03-11-2009 at 06:50 AM.

  6. #6
    Complete Beginner
    Join Date
    Feb 2009
    Posts
    312
    Apart from the comments above, here are some more suggestions:

    1)
    Don't try to apply a personal touch to your code: it's annoying at best. Once you start working on the same piece of code with several other people at once, it will be *really* annoying. If it's a five minute program, don't show it to us in the first place.

    2)
    Always use proper variable names. If you don't learn it right now, you'll have a hard time later. Same goes for all other comments: try to do it right the first time. Programming will become harder once you advance, so learn the easy things right away. Later, there won't be enough time.

    3)
    You don't need that many variables. You need two for the numbers, one for the operation and maybe one for the result. In my opinion, proper names would be "a", "b", "op" and "result". I'd replace sheep, skin et.al. with an enum {add, sub, mul, div} or with #defines.

    4)
    Your functions do similar things, so make them look similar:

    Code:
    int add (int a, int b);
    int dif (int a, int b);
    int mult (int a, int b);
    int div (int a, int b);

    5)
    The function argument's name doesn't need to be the same as in the call to the function. The following code is perfectly legal:

    Code:
    int foo(int a);
    
    [...]
    
    int x;
    foo(x);

    6)
    If you use switch() instead of a series of if-statements, it will make the code much more clear to the reader:

    Code:
    switch(op) {
            case add:
                    printf("%d + %d = %d\n", a, b, add(a, b));
                    break;
            case sub:
                    [...]
    }

    7)
    Sacrifice verbosity for the sake of simplicity: you don't need 8 scanf() statements to read two numbers. Simply ask for the numbers (once) and for the operation to be performed on them, then use the switch to actually do it:

    Code:
    puts("Enter two numbers:");
    scanf("%d\n%d\n", &a, &b);
    puts("1=add, 2=sub, 3=mul, 4=div:");
    scanf("%d\n", &op);
    
    switch(op) {   // ...

    8)
    > Uh, you can't divide nothing.

    Sure you can. 0/x is "0", not "nothing". Your output isn't funny, it merely indicates that you don't know how division works.


    9)
    > I'm at lesson 4, not lesson 104.

    If you don't understand a specific comment or ciriticism, ask for an explanation and try to incorporate it into your way of coding. If you don't show any effort to advance your coding skills, someone else might not be willing to show effort and answer your next question.

    10)
    All of your branches end with getchar(). Simply put the getchar() after the last if-statement.

    11)
    main() has a return type and hence deserves a return value. One might argue that this is not needed since C99, but the general rule is to explicitly write everything down until you know exactly why you can omit it.

    12)
    Don't add meta-information to your code, such as "crappy". Either we're able to conclude it ourselves or it's simply wrong. Use comments for explanations, not excuses.


    If that's your first program, it's better than most first programs I've seen so far.

    Greets,
    Philip
    All things begin as source code.
    Source code begins with an empty file.
    -- Tao Te Chip

  7. #7
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    Quote Originally Posted by laserlight View Post
    In this case int foo() is the same as int foo(void) because it is part of a function definition.
    Are you sure?

    Code:
    int foo()
    {
    	return 0;
    }
    
    int boo(void)
    {
    	return 2;
    }
    
    
    int main() 
    {
    	foo(1);
    	boo(2);
    	return 0;
    }
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  8. #8
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,588
    Quote Originally Posted by vart
    Are you sure?
    hmm... looks like I am wrong on a technicality, since when a function definition acts as a prototype, the point of there being a difference for function declarations comes into play. On the other hand, the underlying point is that a declaration of a function without specifying the parameter list does not declare a different function, thus defining any function, especially main, with a void parameter or with no parameters is effectively the same.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  9. #9
    Registered User
    Join Date
    Mar 2009
    Posts
    7
    1. "Personal touches" are for personal programs, not group stuff. As long as it's mine, I'm gonna sneak something in there. Sorry.

    Furthermore, as I said, it's the first program I've ever made. I'd damn proud of myself, no matter how crappy it is. XD Don't worry, though, the euphoria is wearing off now. I'll have to make an even better program to get it back.

    2. I know, I know. Still, while I'm young, I'm going to goof around a little. As my programs get more and more complex, I'll slap myself upside the head. I always do.

    3. Still learning. Sorry. :P

    4. This is why I showed you my "five-minute" program. So you don't have to give me the same advice fifty programs down the road. Especially when it's advice I can use now.

    5. I think I see where you're getting. "Foo" can be used to manipulate another variable? O.o

    6. Just went on to lesson 5. Love it already. I am going to definitely use it in the future. XD

    Also, it acts like a switch, like a button on your remote, right? I am correct in that interpretation?

    7. Oh, that is some sexy little piece of code. You definitely know what you're doing, unlike me. ==;;

    8. Meh. Like I said, superfluous. I didn't even think it was that funny myself when I wrote it. Which kinda makes it worse. >.>

    9. The point of this wasn't necessarily for expert criticism, but for little things. Also, to brag about my "1337 5|<1LLZ". You can be assured that my future posts will be much more serious.

    10. Good advice. Thanks.

    11. So, include "return 0;" at the end? Just to make sure it looks pretty?

    12. As they said at Leadership Challenge Camp, explanations are just long excuses. :P
    That comment was actually added after people kept bugging me about my unorthodox variables. >.>

    Yes, it is indeed my first program. I did do several of those tutorials an hour earlier, however, even changing them to test the code here and there. The program I have presented here is the first one I made by myself, for myself, with no instruction whatsoever. I took what I knew and decided to do something with it. Unfortunately, you can see what little I did with that knowledge. XD

    EDIT:
    I think I'll begin studying American Government for the time being instead. I still need 2.0 credits for my High School Diploma. I'll kind need that if I want to get into college, and a decent job. I'll pick back up again in a short while (hours, likely).

    -Hiru
    Last edited by Hirumaru; 03-11-2009 at 08:08 AM.

  10. #10
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    Quote Originally Posted by laserlight View Post
    hmm... looks like I am wrong on a technicality, since when a function definition acts as a prototype, the point of there being a difference for function declarations comes into play. On the other hand, the underlying point is that a declaration of a function without specifying the parameter list does not declare a different function, thus defining any function, especially main, with a void parameter or with no parameters is effectively the same.
    Are we talking C or C++?

    in C++
    int f()
    is equivalent of
    int f(void)

    in C

    int f()

    is equivalent of

    int f(...)

    and you have no function overloading in C, so you cannot declare several function with the same name and different parameters in any case
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  11. #11
    C++ Witch laserlight's Avatar
    Join Date
    Oct 2003
    Location
    Singapore
    Posts
    21,588
    Quote Originally Posted by vart
    Are we talking C or C++?
    C. In C++ they are always the same, even for function prototypes.

    Quote Originally Posted by vart
    in C

    int f()

    is equivalent of

    int f(...)
    No, in C, a missing parameter specification in a function declaration merely states that the parameters of the function are not specified.
    C + C++ Compiler: MinGW port of GCC
    Version Control System: Bazaar

    Look up a C++ Reference and learn How To Ask Questions The Smart Way

  12. #12
    Registered User
    Join Date
    Mar 2009
    Posts
    7
    US Government is only slightly less dull than US History. So, I took a little break from my studies and worked a little on my code. Here's the latest iteration:

    Code:
    #include <stdio.h>
    
    int add (int a, int b);
    int dif (int a, int b);
    int mult (int a, int b);
    int div (int a, int b);
    
    int main()
    {
        int a,b;
        int op;
    
        printf( "This is Hiru's crappy calculator.\nPlease choose which crappy operation you would like to perform:\n" );
        printf( "(1) Addition; (2) Subtraction; (3) Multiplication; (4) Division: " );
        scanf( "%d", &op );
    
        switch (op)
        {
        case 1:
            printf( "Enter two numbers to be added (separate w/ space): " );
            scanf( "%d %d", &a, &b );
            printf( "\nYour numbers were %d and %d. Their sum is: %d\n", a, b, add(a,b) );
            break;
        case 2:
            printf( "Enter two numbers to be subtracted (separate w/ space): " );
            scanf( "%d %d", &a, &b );
            printf( "\nYour numbers were %d and %d. Their difference is: %d\n", a, b, dif(a,b) );
            break;
        case 3:
            printf( "Enter two numbers to be multiplied (separate w/ space): " );
            scanf( "%d %d", &a, &b );
            printf( "\nYour numbers were %d and %d. Their product is: %d\n", a, b, mult(a,b) );
            break;
        case 4:
            printf( "Enter two numbers to be divided (separate w/ space): " );
            scanf( "%d %d", &a, &b );
            if ( b == 0 )
                {
                printf( "\nI'm sorry, Dave. I'm afraid I can't do that.\n" );
                getchar();
                }
            else printf( "\nYour numbers were %d and %d. Their quotient is: %d\n", a, b, div(a,b) );
            break;
        default:
            printf( "\nPlease choose only the operations listed. Thank you.\n" );
            break;
            }
        getchar();
        return 0;
    }
    
    int add (int a, int b)
    {    return a + b;  }
    
    int dif (int a, int b)
    {   return a - b;   }
    
    int mult (int a, int b)
    {    return a * b;  }
    
    int div (int a, int b)
    {    return a / b;  }
    Still not spectacular but a little prettier. In an awkward sort of way. >.>

    By the way, as an irrelevant point of little interest, I've been replacing the name in the version I show here. You see my pseudonym while I see my real name. :P

    EDIT: Oops! Oh crap, I forgot to change the variables in the definitions. XD One moment, please . . . . Done!


    -Hiru
    Last edited by Hirumaru; 03-12-2009 at 12:08 AM.

  13. #13
    CSharpener vart's Avatar
    Join Date
    Oct 2006
    Location
    Rishon LeZion, Israel
    Posts
    6,484
    you have a code that repeats itself

    Code:
    #include <stdio.h>
    
    int add (int a, int b);
    int dif (int a, int b);
    int mult (int a, int b);
    int div (int a, int b);
    
    struct operation
    {
    	const char* sDo;
    	const char* sRes;
    	int (*f)(int,int);
    };
    
    int main()
    {
    	int a,b;
    	int op;
    	struct operation ops[] = {
    		{"added" , "sum",&add},
    		{"subtracted" , "difference" , &dif},
    		{"multiplied" , "product" , &mult},
    		{"divided" , "quotient" , &div}
    	};
    	printf( "This is Hiru's crappy calculator.\nPlease choose which crappy operation you would like to perform:\n" );
    	printf( "(1) Addition; (2) Subtraction; (3) Multiplication; (4) Division: " );
    	scanf( "%d", &op );
    
    	switch (op)
    	{
    	case 1:
    	case 2:
    	case 3:
    	case 4:
    		printf( "Enter two numbers to be %s (separate w/ space): " , ops[op-1].sDo);
    		scanf( "%d %d", &a, &b );
    		if (op == 4 &&  b == 0 )
    		{
    			printf( "\nI'm sorry, Dave. I'm afraid I can't do that.\n" );
    			getchar();
    		}
    		else
    		{
    			printf( "\nYour numbers were %d and %d. Their %s is: %d\n", a, b, ops[op-1].sRes,ops[op-1].f(a,b) );
    		}
    		break;
    	default:
    		printf( "\nPlease choose only the operations listed. Thank you.\n" );
    		break;
    	}
    	getchar();
    	return 0;
    }
    
    int add (int a, int b)
    {    return a + b;  }
    
    int dif (int a, int b)
    {   return a - b;   }
    
    int mult (int a, int b)
    {    return a * b;  }
    
    int div (int a, int b)
    {    return b / b;  }
    The first 90% of a project takes 90% of the time,
    the last 10% takes the other 90% of the time.

  14. #14
    Registered User
    Join Date
    Oct 2008
    Posts
    98
    Quote Originally Posted by vart View Post
    you have a code that repeats itself

    Code:
    #include <stdio.h>
    
    int add (int a, int b);
    int dif (int a, int b);
    int mult (int a, int b);
    int div (int a, int b);
    
    struct operation
    {
    	const char* sDo;
    	const char* sRes;
    	int (*f)(int,int);
    };
    
    int main()
    {
    	int a,b;
    	int op;
    	struct operation ops[] = {
    		{"added" , "sum",&add},
    		{"subtracted" , "difference" , &dif},
    		{"multiplied" , "product" , &mult},
    		{"divided" , "quotient" , &div}
    	};
    	printf( "This is Hiru's crappy calculator.\nPlease choose which crappy operation you would like to perform:\n" );
    	printf( "(1) Addition; (2) Subtraction; (3) Multiplication; (4) Division: " );
    	scanf( "%d", &op );
    
    	switch (op)
    	{
    	case 1:
    	case 2:
    	case 3:
    	case 4:
    		printf( "Enter two numbers to be %s (separate w/ space): " , ops[op-1].sDo);
    		scanf( "%d %d", &a, &b );
    		if (op == 4 &&  b == 0 )
    		{
    			printf( "\nI'm sorry, Dave. I'm afraid I can't do that.\n" );
    			getchar();
    		}
    		else
    		{
    			printf( "\nYour numbers were %d and %d. Their %s is: %d\n", a, b, ops[op-1].sRes,ops[op-1].f(a,b) );
    		}
    		break;
    	default:
    		printf( "\nPlease choose only the operations listed. Thank you.\n" );
    		break;
    	}
    	getchar();
    	return 0;
    }
    
    int add (int a, int b)
    {    return a + b;  }
    
    int dif (int a, int b)
    {   return a - b;   }
    
    int mult (int a, int b)
    {    return a * b;  }
    
    int div (int a, int b)
    {    return b / b;  }
    Throwing in structs and pointers is jumping the gun quite a bit here, no? This person is on their first program... While structs and pointers are wonderful things, they aren't the right thing to put in this post.

  15. #15
    Registered User
    Join Date
    Mar 2009
    Posts
    7
    As strange as it sounds, all that code actually makes sense to me. O.o

    I may not understand how every little piece in it works but I can see how it's working in that code. Even so, I still don't have much of a clue as to how a pointer works yet. Even though I've already read that tutorial and can see how useful they can be. @_@

    Oh, and a question. This is the basic purpose for creating a function, more or less, right?
    Code:
    #include whatever
    
    Do_This();
    
    int main()
    {
    
        printf( "Make your choice:\n );
    
        /* Which leads to . . . */
    
        Do_This();
    
    }
    
    
    Do_This()
    {
    
        /* Whatever was intended to be done. */
    
    }
    EDIT: Main() doesn't and shouldn't handle everything. As the programs get more complex, I should be putting less and less in main() and using more and more independent functions. Right? If so, I can see how everything works now. Main() starts the program, then the program takes it from there. Right?

    -Hiru
    Last edited by Hirumaru; 03-12-2009 at 12:26 AM.

Page 1 of 2 12 LastLast
Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Issue with program that's calling a function and has a loop
    By tigerfansince84 in forum C++ Programming
    Replies: 9
    Last Post: 11-12-2008, 12:38 PM
  2. Need help with a program, theres something in it for you
    By engstudent363 in forum C Programming
    Replies: 1
    Last Post: 02-29-2008, 12:41 PM
  3. Replies: 4
    Last Post: 02-21-2008, 09:39 AM
  4. My program, anyhelp
    By @licomb in forum C Programming
    Replies: 14
    Last Post: 08-14-2001, 10:04 PM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21