Looking for some feedback...

This is a discussion on Looking for some feedback... within the C++ Programming forums, part of the General Programming Boards category; Hey all, I've just finished my horsepower calculator and am looking for input. Since everyone I know who knows C/C++ ...

  1. #1
    Registered User dead_cell's Avatar
    Join Date
    Jun 2002
    Posts
    44

    Post Looking for some feedback...

    Hey all,

    I've just finished my horsepower calculator and am looking for input. Since everyone I know who knows C/C++ doesn't check their emails for a week or so, I thought this was the most efficient route for getting feedback. The source is attached and needs to be compiled (It works in both MSVC++ .NET and DevCPP 4, so it should compile for most other compilers).

    It works on the premise that the user is finding the output at the flywheel and that they are finding both the horsepower and torque (foot-pounds) at a specific RPM output (meaning that the numbers won't match up exactly to factory-rated specs because normally the torque peak is at a lesser RPM than the HP peak is). I'm thinking about doing more calcs for different engine dynamics based on what I get here, so please don't hesitate to give your input.
    Linux is great - It does infinite loops in five seconds!

    ~Linus Torvalds

  2. #2
    Registered User dead_cell's Avatar
    Join Date
    Jun 2002
    Posts
    44

    Arrow oops, forgot the attachment

    oops, I forgot to attach the source... ( ) here, it should work now
    Attached Files Attached Files
    Linux is great - It does infinite loops in five seconds!

    ~Linus Torvalds

  3. #3
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,439
    1. replace the stars() and wait() macros with functions. There is no reason in C++ to use macro-like functions.
    In C, function-like macros are typically used where performance is critical, but in C++ this is achieved by using inline functions. Inline functions offer the speed improvements without the nasty side effects which you can get with function-like macros.

    2. Tidy up the indentation - it looks a bit ragged in a few places.

    3. Your global vars should be local to each function. Global variables should be avoided where possible.

    4. 5252 is a magic number. Define it as
    const int magic = 5252;
    But give it a proper name, and a comment.

    5. The while loop in main is very long, and adding more features will make it longer still. Consider shortening it by making the list of options into a function call, and moving the switch statement to another function
    Eg.
    Code:
    void menu ( void ) {
        cout<<"What operation do you wish to perform?"<<endl<<endl
            <<"1 - Horsepower @ RPM"<<endl
            <<"2 - Torque @ RPM"<<endl
            <<"3 - RPM based on horsepower & torque output"<<endl
            <<"4 - Version info"<<endl
            <<"0 - Quit"<<endl<<endl;
    }
    void action ( int choice ) {
        switch (choice)
        {
        case 1 : hp_rpm();
            break;
        case 2 : tq_rpm();
            break;
        case 3 : hp_tq();
            break;
        case 4 : ver_info();
            break;
        default:
            cout<<"Error, bad input, quitting"<<endl<<endl;
            break;  // good idea for default to have break as well
        }
    }
    
    In main
        menu();
        cin >> choice;  // this should really be validated
        if ( choice != 0 ) {
            action(choice);
        } else {
            cout<<"Please wait, halting program..."<<endl<<endl;
            wait()
            return 0;
        }

  4. #4
    Registered User dead_cell's Avatar
    Join Date
    Jun 2002
    Posts
    44

    Talking THANKS!!!

    Thanks, Salem. I'm not a wizard with C++ (as you can see ), but automobile mathematics has been said to be my forte'... Like I said in my original post, I'm thinking on coding some more like this only just a little more robust... (engine dynamics and aerodynamics for calculating intake curve, injector pulse, etc...)

    I think more reading on C++ programming ethics is in order. Just because it compiles and works doesn't mean it is completely correct.
    Linux is great - It does infinite loops in five seconds!

    ~Linus Torvalds

  5. #5
    Registered User dead_cell's Avatar
    Join Date
    Jun 2002
    Posts
    44

    BTW:

    by the way, I'm compiling this in MSVC++.NET so the indentation is a little awry in some places... Sorry for any inconveniences
    Linux is great - It does infinite loops in five seconds!

    ~Linus Torvalds

  6. #6
    Registered User dead_cell's Avatar
    Join Date
    Jun 2002
    Posts
    44

    A little problem... tell me if I'm doing it right

    I've split the menu and the actual action functions, but I encountered a warning - the app still compiled correctly - it's just that warnings annoy me and the fact that the code won't compile correctly on some other compilers leaves that irking feeling at the back of my mind. Anyway, I've narrowed it down to one main function, being the action() or switchmain() as I've labelled it. The error says

    Code:
    warning C4715: 'switchmain' : not all control paths return a value
    Now, I'm thinking that since it has a switch scenario inside the function returning a value (case 0 : return 0) and that anything outside the case scenario is just being left to void(). Since I can't just go and label it void switchmain() because the function itsself is returning a 0 in one part of it, can't I just place a return true; outside the case scenario to remedy that, just as long as I keep the function initiator to something like short int/int switchmain()?

    Here's the code:

    Code:
    short int switchmain(int choice)
    {
    	switch (choice)
    	{
    	case 1 : hp_rpm();
    		break;
    	case 2 : tq_rpm();
    		break;
    	case 3 : hp_tq();
    		break;
    	case 4 : ver_info();
    		break;
    	case 0 : 
    		cout<<"Please wait, halting program..."<<endl<<endl;
    		wait();
    			return 0;;
    		break;
    	default:
    		cout<<"Error, bad input, quitting"<<endl<<endl;
    	}
    	return true;
    }
    I just want to know if I'm doing this right
    Linux is great - It does infinite loops in five seconds!

    ~Linus Torvalds

  7. #7
    elad
    Guest
    Depends--if one of the cases has a return, they all should, _unless_ you call the menu function to make another choice from within the functions called in the switch from cases other than case 0--I didn't look further into your code than the last post so I don't know how you handle it. In the first scenario you could have everything but case 0 return a 1.

    The value 0 is frequently equated to the value false in type boolean, and values not 0 to true.

  8. #8
    and the hat of wrongness Salem's Avatar
    Join Date
    Aug 2001
    Location
    The edge of the known universe
    Posts
    32,439
    > warning C4715: 'switchmain' : not all control paths return a value
    Probably caused by the return inside the switch, which makes the break; unreachable.

    Try something like
    Code:
    bool switchmain(int choice)
    {
        bool result = true;
        switch (choice)
        {
        case 1 : hp_rpm();
            break;
        case 2 : tq_rpm();
            break;
        case 3 : hp_tq();
            break;
        case 4 : ver_info();
            break;
        case 0 : 
            cout<<"Please wait, halting program..."<<endl<<endl;
            wait();
            result = false; // flag we want to exit now
            break;
        default:
            cout<<"Error, bad input, quitting"<<endl<<endl;
        }
        return result;
    }

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Looking for feedback on program segment
    By avron in forum C++ Programming
    Replies: 4
    Last Post: 05-07-2007, 04:38 PM
  2. Feedback?
    By CornedBee in forum A Brief History of Cprogramming.com
    Replies: 8
    Last Post: 01-02-2004, 10:01 PM
  3. Feedback wanted - DispHelper
    By anonytmouse in forum A Brief History of Cprogramming.com
    Replies: 3
    Last Post: 12-31-2003, 08:30 AM
  4. Feedback for my PacMan clone
    By snoman in forum Game Programming
    Replies: 2
    Last Post: 06-23-2003, 05:15 PM
  5. Primary game build, feedback welcome
    By Clyde in forum Game Programming
    Replies: 37
    Last Post: 07-10-2002, 05:34 AM

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