LinuxQuestions.org

LinuxQuestions.org (/questions/)
-   Programming (https://www.linuxquestions.org/questions/programming-9/)
-   -   Just looking for some judgement and/or advice on making it better. (https://www.linuxquestions.org/questions/programming-9/just-looking-for-some-judgement-and-or-advice-on-making-it-better-830530/)

jmc1987 09-05-2010 09:54 PM

Just looking for some judgement and/or advice on making it better.
 
I just started coding c++ like 3 days ago so I am moving on. I wrote a little more advanced script and I just wanted to know your guys judgement and advice on the code for a newbie coding it ;)

Code:

// Temperature convertor Created by JMC1987
//You are here by allowed to modify and use the code as legal by law.
//coded in Code::Blocks 8.02
#include <iostream>
using namespace std;
float degreeFahrenheit(float receivedFah){
    cout << receivedFah << " Now being converted into Celsius.\n";
    // Equation to convert into Celsius
    float celsuis;
    return (celsuis = ((receivedFah - 32 ) * 5)/9);
}
float degreeCelsuis(float receivedCel){
    cout << receivedCel << " Now being converted into Fahrenheit.\n";
    float fahrenheit;
    return (fahrenheit =((receivedCel * 9) /5) + 32);
    }



int main(){

    cout << "Welcome to JC Temperature convertor\n";
    cout << "You may use this program any ways you please as long as its legal by law.\n";
    cout << endl;
    cout << "What Tempature would you like to Convert?\n";
    cout << "Please type 1 for Fahrenheit or 2 for Celsius.\n";
    cout << "Celsuis or Fahrenheit?: ";

    int temp;
    cin >> temp;

    if ( temp == 1){
        cout << "You chose Fahrenheit.\n ";
        cout << endl;
        cout << "Please type the Degrees Fahrenheit you would like convert into celsius.\n ";
        cout << "\nFahrenheit: ";
        float tempf, tempc;
        cin >> tempf;
        tempc = degreeFahrenheit(tempf);
        cout << endl;
        cout << tempf << " Degrees Fahrenheit is equal to " << tempc << " Degreess Celsuis. \n";

    }
    else if (temp == 2){
        cout << "You Chose Celsius.\n ";
        cout << endl;
        cout << "Please type the Degrees Celsius you would like convert into Fahrenheit.\n";
        cout << "\nCelsuis: ";
        float tempc, tempf;
        cin >> tempc;
        tempf = degreeCelsuis(tempc);
        cout << endl;
        cout << tempc << " Degress Celsuis is equal to " << tempf << " Degrees Fahrenheit.\n";
            }
    else {
        cout << "You typed an invalid entry.  Please rerun with either option 1 or 2\n ";

    }
    cout << "\nThank you for using JMC1987 Temperature calculater.\n ";


    return 0;
}


Sergei Steshenko 09-05-2010 10:04 PM

A good habit is to use 'unsigned' instead of 'int' whenever possible. In case of this program it is possible.

It would make more sense fro end user point of view to use f/c instead of 1/2 as characters selecting input temperature type.

It would be a good habit to use FP numbers instead of integer one in formulas whose end result is FP.

grail 09-05-2010 10:23 PM

Well there are 2 things I like to do (so may only be me):

1. Create and / or initialise variables at the start of functions / code

eg. In main you create your float variables inside the if statements, but you do it twice. So start of main for me would look like:
Code:

int main()
{
    int temp;
    float tempf, tempc;

This helps in 2 ways:

a. You don't have to search throughout my code to find variable types as they are always located in the same place.
b. Compared to yours, no need for useless second invocation of float creation.

2. I am a bit of a nazi when it comes to repetition of code. In your example this would be the replication of all the code in your if's.
Your code:
Code:

if ( temp == 1){
        cout << "You chose Fahrenheit.\n ";
        cout << endl;
        cout << "Please type the Degrees Fahrenheit you would like convert into celsius.\n ";
        cout << "\nFahrenheit: ";
        float tempf, tempc;
        cin >> tempf;
        tempc = degreeFahrenheit(tempf);
        cout << endl;
        cout << tempf << " Degrees Fahrenheit is equal to " << tempc << " Degreess Celsuis. \n";

    }
    else if (temp == 2){
        cout << "You Chose Celsius.\n ";
        cout << endl;
        cout << "Please type the Degrees Celsius you would like convert into Fahrenheit.\n";
        cout << "\nCelsuis: ";
        float tempc, tempf;
        cin >> tempc;
        tempf = degreeCelsuis(tempc);
        cout << endl;
        cout << tempc << " Degress Celsuis is equal to " << tempf << " Degrees Fahrenheit.\n";
            }
    else {
        cout << "You typed an invalid entry.  Please rerun with either option 1 or 2\n ";

    }

My alternative:
Code:

float tempin, tempout;
string from, to;
bool valid = true;

if ( temp == 1)
{
    from = "Fahrenheit";
    to = "Celsuis";
    cin >> tempin;
    tempout = degreeFahrenheit(tempin);
}
else if (temp == 2)
{
    from = "Fahrenheit";
    to = "Celsuis";
    cin >> tempin;
    tempout = degreeCelsuis(tempin);
}
else
{
    cout << "You typed an invalid entry.  Please rerun with either option 1 or 2\n ";
    valid = false;
}

if (valid)
{
    cout << "You chose "<<from<<endl;
    cout << endl;
    cout << "Please type the Degrees "<<from<<" you would like convert into "<<to<<endl;
    cout << from<<": ";
    cout << endl;
    cout << tempin << " Degrees "<<from<<" is equal to " << tempout << " Degreess "<<to<<endl;
}

This untested but you get the rough idea.

Sergei Steshenko 09-05-2010 10:30 PM

Quote:

Originally Posted by grail (Post 4089111)
...
1. Create and / or initialise variables at the start of functions / code
...

That is highly debatable. I prefer to follow "create objects as late as possible and get rid of them as early as possible" philosophy which comes natural in languages with lexical scope (e.g. C/C++/Perl).


All times are GMT -5. The time now is 08:11 PM.