Skip to main content
gamedev.net
🔒 Locked

Simplifying Code

Started by highlander411 5 replies 1.2k views
Original Post
highlander411
highlander411
I'm trying to simplify this bloated code for a function based Fahrenheit | Celsius conversion. #include <iostream> #include <cmath> using namespace std; double fTemp = 0.0f; double cTemp = 0.0f; int select = 0; double celsius_to_fahrenheit(double, double); double fahrenheit_to_celsius(double, double); const static float fiveNineths = 5.0F/9.0F; const static float nineFifths = 9.0F/5.0F; float fahrenheit_to_celsius( const float fTemp ) { return (fiveNineths * (fTemp - 32.0F)); } float celsius_to_fahrenheit( const float cTemp ) { return ( (nineFifths * cTemp) + 32.0F); } int main() { while (select ==0) { cout << "Please select from the following: \n"; cout << "Fahrenheit-to-Celsius [1], Celsius-to-Fahrenheit [2], Quit 3 \n"; cin >> select; if (select == 1) { cout << "Enter temperature in degree F to convert: "; cin >> fTemp; //passing the fTemp to the function cout << "The temperature in F, "<< fTemp <<" is " << fahrenheit_to_celsius(fTemp) <<" C\n\n"; } else if (select == 2) { cout <<"Enter temperature in degree C to convert: "; cin >> cTemp; //passing the CIN >> cTemp to the other function then calling it cout << "The temperature in C, "<< cTemp <<" is " << celsius_to_fahrenheit(cTemp) <<" F\n\n"; } else if (select == 3) exit(0); else cout << "Incorrect choice. Try again. \n\n"; select = 0; } system("PAUSE"); return EXIT_SUCCESS; }
Aardvajk
Aardvajk
Doesn't look that bloated to me, but:

  • You don't need any of those variables to be global - locals declared just before use will do instead.
  • You don't need to prototype a function if the function is fully defined before use (your prototypes don't match the functions anyway).
  • There is limited point in defining your FiveNinths etc as constants when they are only used once.


    • #include <iostream>#include <cmath>using namespace std;float fahrenheit_to_celsius( const float fTemp ){    return ((5.0f/9.0f) * (fTemp - 32.0F));}float celsius_to_fahrenheit( const float cTemp ){    return ( ((9.0f/5.0f) * cTemp) + 32.0F);}int main(){    int select=0;    while(select==0)        {        cout << "Please select from the following: \n";        cout << "Fahrenheit-to-Celsius [1], Celsius-to-Fahrenheit [2], Quit 3 \n";        cin >> select;        if (select == 1)            {            cout << "Enter temperature in degree F to convert: ";                        float fTemp;            cin >> fTemp;            cout << "The temperature in F, "<< fTemp <<" is " << fahrenheit_to_celsius(fTemp) <<" C\n\n";            }        else if (select == 2)            {            cout <<"Enter temperature in degree C to convert: ";            float cTemp;            cin >> cTemp; //passing the CIN >> cTemp to the other function then calling it            cout << "The temperature in C, "<< cTemp <<" is " << celsius_to_fahrenheit(cTemp) <<" F\n\n";            }        else if (select == 3) exit(0);        else cout << "Incorrect choice. Try again. \n\n";        select = 0;        }    system("PAUSE");    return EXIT_SUCCESS;}


    Your comments alongside the cin>> lines suggests that you maybe misunderstand about passing parameters to functions. Your cTemp in the main function is completely unrelated to the cTemp in the parameter list of celcius_to_farenheight().

    void f(int x){    x+=10;}int main(){    int a=10;    f(a);}


    In the above, when f() is called, a copy of 'a' is made and put in 'x'. 'x' lives for the duration of the function then vanishes. Any changes to 'x' within f() do not affect 'a' in any way at all.

    In your code, you happen to have named the function parameters the same as the local (or global in your original code) variables - they do not have any other relationship to each other other than the name - they are entirely different variables.

    HTH
    emiel1
    emiel1
    Also I would use a switch, instead of the if else if, etc.

    while (select == 0){   cout << "Please select from the following: \n";   cout << "Fahrenheit-to-Celsius [1], Celsius-to-Fahrenheit [2], Quit [3] \n";   cin >> select;   switch (select)   {   case 1:      {         cout << "Enter temperature in degree F to convert: ";         cin >> fTemp;         cout << "The temperature in F, "<< fTemp <<" is " << fahrenheit_to_celsius(fTemp) <<" C\n\n";         select = 0;      } break;   case 2:      {         cout <<"Enter temperature in degree C to convert: ";         cin >> cTemp;         cout << "The temperature in C, "<< cTemp <<" is " << celsius_to_fahrenheit(cTemp) <<" F\n\n";         select = 0;      } break;   case 3:      {         continue;      } break;   default:      {         cout << "Incorrect choice. Try again. \n\n";         select = 0;      } break;   }}


    The select = 0 is to repeat the question before checking if it is zero (by the while loop.
    Continue will spin to the while and the check will fail (because select is 3, checked by the switch. The while will fail and the rest of the program will execute.

    Emiel1
    highlander411
    highlander411
    Ok, got it. I see what your saying. I think I heard that but dismissed it. Thanks! That helps
    Nyarlath
    Nyarlath
    Quote:
    Original post by EasilyConfused
  • There is limited point in defining your FiveNinths etc as constants when they are only used once.


    • They are used once only, but they should not! The factors in the two functions must be the same.

    static const float ctf_factor = 9.0f / 5;static const float ctf_offset = 32;float fahrenheit_to_celsius( const float fTemp ){    return (1 / ctf_factor) * (fTemp - ctf_offset);}float celsius_to_fahrenheit( const float cTemp ){    return ctf_factor * cTemp + ctf_offset;}

    highlander411
    highlander411
    Nyarlath:

    I don't understand. Need to use what twice? To function I thought I only executed one or the other at a time?.
    Brother Bob
    Brother Bob
    Quote:
    Original post by highlander411
    Nyarlath:

    I don't understand. Need to use what twice? To function I thought I only executed one or the other at a time?.


    The purpose of giving constants names is to describe the prupose, and not have magic numbers spread over the code. Doing const float two = 2; is just as much a magic number as writing 2 in the code. There is no information provided why 2 is used.

    The factor used during conversion is, in fact, just one factor. If one factor is 9/5, then the other one is 5/9. There is a complete dependence on the two. So what he does is defining just one factor, ctf_factor, and giving it a descriptive name at the same time.

    The name fiveNineths gives no information that 5/9 doesn't, while ctf_factor suggests it is the factor for C to F conversion. Further, that factor is also used when converting the other way around, since it must be the same constant. The OP's code suggests that there are two different factors, when in fact there is just one, becuase of the complete dependence between them.

    Topic Locked

    This topic has been locked by a moderator. New replies are not allowed.

    Sign in to reply to this topic.