Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 3 of 3
  1. #1
    New to the CF scene
    Join Date
    Oct 2011
    Posts
    3
    Thanks
    1
    Thanked 0 Times in 0 Posts

    Does this look like good design, or wasteful?

    Just would like an opinion on whether this is good design or not.
    Im trying to tuck things away into their own functions, and keep to good programming techniques.
    What do you think?


    Code:
    #include <stdio.h>
    
    
    void Menu()
    {
        printf("\nChoose an option.\n");
        printf("\n1:Print name.");
        printf("\n2:Print time.");
        printf("\n3:Input number between 1 - 50, and list.");
        printf("\n4:Quit\n");
    
        return;
    }
    
    
    int Input(int &a)
    {
        scanf("%d",&a);
        return a;
    
    }
    
    
    void PrintName()
    {
        printf("\nCodeGibbon\n");
        return;
    
    }
    
    
    void PrintTime()
    {
        printf("\nTime: Now\n");
        return;
    
    }
    
    void CheckInput(int &a, int &z)
    {
        if (a >= 0 && a <= 50)
        {
            printf("Valid Input\n");
            z = 1;
            return;
        }
        printf("Invalid Input\n");
        z = 0;
        return;
    }
    
    
    void ListNumbers()
    {
    
        int b = 0;
        int z;
        printf("\nEnter Number between 1 and 50\n");
        b = Input(b);
        CheckInput(b,z);
    
    
    
        if (z == 1)
        {
    
    
        for (int i = 0; i <= b; i++ )
        {
            printf("%d ", i);
    
    
        }
        }
    
    
    return;
    
    }
    
    
    
    
    int main()
    {
        int a;
    
        while (a != 4)
        {
    
    
    
        Menu();
        a = Input(a);
    
    
        switch(a)
        {
            case 1:
            PrintName();
    
            break;
    
    
            case 2:
            PrintTime();
    
            break;
    
            case 3:
            ListNumbers();
    
            break;
    
            //case 4:
    
           // break;
        }
    
    
    
    
    
        }
    
        return 0;
    }

  • #2
    Rockstar Coder
    Join Date
    Jun 2002
    Location
    USA
    Posts
    9,074
    Thanks
    1
    Thanked 328 Times in 324 Posts
    Your "Input" function seems to be overkill, just use the scanf directly instead. Maybe if you were collecting multiple inputs and validating them it should be in its own function but as it stands, I wouldn't keep it.

    I would also make the CheckInput function return if the input is valid or not instead requiring the output variable to passed in. And if you did that I would rename the function to "IsInputValid" to better convey what the function is determining.

    I also notice you are using pass by reference which means you must be compiling this as C++ code. If you are doing that why not make your life a little easier and using cin and cout instead of printf and scanf? If you are actually going to stick with using C++ code there would be a few other things I would change as well.

    Or pass by pointer and compile as C code?
    OracleGuy

  • #3
    New Coder
    Join Date
    Oct 2011
    Posts
    10
    Thanks
    1
    Thanked 0 Times in 0 Posts
    I always put comments in my code so that anyone else who happens to read it will know what the hell is going on without having to squint their eyes to read every single line of code. That seems to be a good structural technique to add in.


  •  

    Posting Permissions

    • You may not post new threads
    • You may not post replies
    • You may not post attachments
    • You may not edit your posts
    •