Sign in to follow this  
SamuelRS

Problem with functions... I think

Recommended Posts

Well, basically I have been doing c++ for about 2 months now. And I have just learnt how to do basic functions. But, knowing me, I've decided to hurdle before I can crawl. So, I have tried to create a temperature converter. It seems to work all the way up to the point when it actually needs to do the math, I think it has something to do with the variable types, as VS.net lets me compile and run the app, but gives me warnings about truncating. But, then again, nothing like this has happened before I started using functions, so I don't know which is going wrong. Anyways, heres the source: [CODE = C++] #include <iostream> using namespace std; int getFtemp(); //function prototypes int getCtemp(); int main() { char letter; cout << "Please insert either \'c\' to convert FROM Celsius or \'f\' to convert FROM fahrenheit: "; cin >> letter; cin.ignore(); switch(letter) { case 'c' : getCtemp(); break; case 'f' : getFtemp(); break; default : cout << "ERROR: You did not enter either \'c\' or \'f\'"; } cin.ignore(); cin.get(); } int getCtemp() { float Icelsius; int Rfahrenheit; cout << "Enter a celsius temperatur that you want to convert into fahrenheit: "; cin >> Icelsius; cin.ignore(); Rfahrenheit = (((Icelsius*9)/5)+32); return (int) Rfahrenheit; } int getFtemp() { float factor = 5.0 / 9.0; float freezing = 32.0; int Rcelsius; float ftemp; cout << "Enter the fahrenheit temperature that you want to convert into celsius: "; cin >> ftemp; cin.ignore(); Rcelsius = factor * (ftemp - freezing); return (int) Rcelsius; } [/CODE] Thanks for reading, Samuel

Share this post


Link to post
Share on other sites
Firstly, it is refreshing to see someone trying such a nice simple application when starting out.

Secondly, it would be helpful if you could specify the WAY in which your code is not working.

With this in mind, the only immediatley obvious things to me are that you are not doing anything with the returns from the functions, so they are not being output at all. Also, the truncating warnings will be to do with converting from floats to ints. Could you not just use floats throughout the code, as the return values from your functions? I would have thought the final result would want to be a float as well.

Sorry if this does not help and if you could post with exactly what the problem is, it would be more useful.

[EDIT] Faaar to slow.

Share this post


Link to post
Share on other sites
Well, first of all, thanks for replying. The thing thats not working is that, when the user enters the values to be converted, the program doesn't give an answer, it just leaves a blank gap.

EasilyConfused: I get what your saying, but I am unable to get that into the code. Here is the new code (all the changes are in the function definitions).

#include <iostream>
using namespace std;

int getFtemp(); //function prototypes
int getCtemp();

int main()
{
char letter;

cout << "Please insert either \'c\' to convert FROM Celsius or \'f\' to convert FROM fahrenheit: ";

cin >> letter;
cin.ignore();

switch(letter)
{
case 'c' : getCtemp(); break;
case 'f' : getFtemp(); break;
default : cout << "ERROR: You did not enter either \'c\' or \'f\'";
}

cin.ignore();
cin.get();
}

int getCtemp()
{
float Icelsius;
int Rfahrenheit;

cout << "Enter a celsius temperatur that you want to convert into fahrenheit: ";
cin >> Icelsius;
cin.ignore();

Rfahrenheit = (((Icelsius*9)/5)+32);

return (float) Rfahrenheit;

cout << Rfahrenheit << "F";
}

int getFtemp()
{
float factor = 5.0 / 9.0;
float freezing = 32.0;
int Rcelsius;
float ftemp;

cout << "Enter the fahrenheit temperature that you want to convert into celsius: ";
cin >> ftemp;
cin.ignore();

Rcelsius = factor * (ftemp - freezing);

return (float) Rcelsius;

cout << Rcelsius << "C";
}

EDIT: EkimGram - I didn't think that it was neccessary.

Share this post


Link to post
Share on other sites
You are nearly there dude. You just have your cout << result; statements after your return statements, so they never get executed.


int getCtemp()
{
float Icelsius;
int Rfahrenheit;

cout << "Enter a celsius temperatur that you want to convert into fahrenheit: ";
cin >> Icelsius;
cin.ignore();

Rfahrenheit = (((Icelsius*9)/5)+32);

cout << Rfahrenheit << "F" << endl;

return (float) Rfahrenheit;
}

int getFtemp()
{
float factor = 5.0 / 9.0;
float freezing = 32.0;
int Rcelsius;
float ftemp;

cout << "Enter the fahrenheit temperature that you want to convert into celsius: ";
cin >> ftemp;
cin.ignore();

Rcelsius = factor * (ftemp - freezing);

cout << Rcelsius << "C" << endl;

return (float) Rcelsius;
}




OR


int main()
{
char letter;

cout << "Please insert either \'c\' to convert FROM Celsius or \'f\' to convert FROM fahrenheit: ";

cin >> letter;
cin.ignore();

switch(letter)
{
case 'c' : cout << getCtemp() << endl; break;
case 'f' : cout << getFtemp() << endl; break;
default : cout << "ERROR: You did not enter either \'c\' or \'f\'";
}

cin.ignore();
cin.get();
}




Note the endl at the ends of the cout lines, to print a newline.

HTH

[EDIT] return 0; isn't STRICTLY necessary since it is considered explicit at the end of main (and only main) if it is not included. Many people (including me) do consider it bad form though, since it can encourage you to forget it in other functions where there is no implicit return.

Share this post


Link to post
Share on other sites
Quote:

int getCtemp()
{
float Icelsius;
int Rfahrenheit;

cout << "Enter a celsius temperatur that you want to convert into fahrenheit: ";
cin >> Icelsius;
cin.ignore();

Rfahrenheit = (((Icelsius*9)/5)+32);

return (float) Rfahrenheit;

cout << Rfahrenheit << "F";
}

int getFtemp()
{
float factor = 5.0 / 9.0;
float freezing = 32.0;
int Rcelsius;
float ftemp;

cout << "Enter the fahrenheit temperature that you want to convert into celsius: ";
cin >> ftemp;
cin.ignore();

Rcelsius = factor * (ftemp - freezing);

return (float) Rcelsius;

cout << Rcelsius << "C";
}

I've bolded the problem. Your function is ending before it gets to print the values. Put the return statements after the cout <<'s

Edit: beaten to it...by several minutes. Darn.

Edit Edit: Removed unnecessary change.

[Edited by - DigiDude on July 20, 2006 12:14:15 PM]

Share this post


Link to post
Share on other sites
Well, I finially got it working. So thanks to all you guys! If this program ever becomes a success I will make sure to mention you (next stop: Buying Microsoft).

Anyway, my final code:


#include <iostream>
using namespace std;

int getFtemp(); //function prototypes
int getCtemp();

int main()
{
char letter;

cout << "Please insert either \'c\' to convert FROM Celsius or \'f\' to convert FROM fahrenheit: ";

cin >> letter;
cin.ignore();

switch(letter)
{
case 'c' : getCtemp(); break;
case 'f' : getFtemp(); break;
default : cout << "ERROR: You did not enter either \'c\' or \'f\'";
}

cin.get();
return 0; //Added return to bottom of code, after all I don't want to have bad form do I ;)
}

int getCtemp()
{
float Icelsius;
float Rfahrenheit; // Vhangeed to float

cout << "Enter a celsius temperatur that you want to convert into fahrenheit: ";
cin >> Icelsius;
cin.ignore();

Rfahrenheit = (((Icelsius*9)/5)+32);

cout << Rfahrenheit << "F"; // Moved above return

return (float) Rfahrenheit;
}

int getFtemp()
{
float factor = 5.0 / 9.0;
float freezing = 32.0;
float Rcelsius; // Changed float variable
float ftemp;

cout << "Enter the fahrenheit temperature that you want to convert into celsius: ";
cin >> ftemp;
cin.ignore();

Rcelsius = factor * (ftemp - freezing);

cout << Rcelsius << "C"; // Moved above return

return (float) Rcelsius;

}




Thanks again,
Samuel

EDIT: The program doesn't work again, when I place the return at the end of the main program. Is it supposed to go before or after cin.get();

Share this post


Link to post
Share on other sites
Ekim_Gram: Not necessary. Yes, this is standard.

DigiDude: Your "correcion" to the arithmetic is quite incorrect.

To the OP: You don't need to declare variables for everything. You can return an expression. Casts apply to expressions as well.

You also do not have to put your variable declarations at the top of scope, and it is recommended that you not do so. Instead, put them near first use, and when possible, initialize them with their first value. Initialization is different from assigning.


// BAD
int x;
do_stuff_not_involving_x();
x = 5;

// ALSO BAD
int x = 0;
do_stuff_not_involving_x();
x = 5;

// GOOD
do_stuff_not_involving_x();
int x = 5;

Share this post


Link to post
Share on other sites
Quote:
Original post by Zahlman
You also do not have to put your variable declarations at the top of scope, and it is recommended that you not do so. Instead, put them near first use, and when possible, initialize them with their first value. Initialization is different from assigning.


// BAD
int x;
do_stuff_not_involving_x();
x = 5;

// ALSO BAD
int x = 0;
do_stuff_not_involving_x();
x = 5;

// GOOD
do_stuff_not_involving_x();
int x = 5;



I will take that into account next time.

Share this post


Link to post
Share on other sites
You're using functions in a wrong fashion. Don't put input/output code in them. Right now, you have functions that do both the calculations and the I/O, and their return values aren't used, because they do their own printing of the results. You should have FarhToCels() and CelsToFarh() that just do the conversions and handle I/O elsewhere, like in the main program. For example:


cout << "Insert Farhennheit: ";
float Ftemp;
cin>>Ftemp;
float Ctemp=FarhToCels(Ftemp);
cout<< Ftemp<<"F"<<" equals to "<<Ctemp<<"C"<<endl;

Share this post


Link to post
Share on other sites
Also, you are working with floats in your functions. The functions should be returning floats, not ints. I believe you mentioned something about "loss of precision" earlier -- this is the reason for it. Floating point numbers are more precise than integers are. By returning an int, you are discarding the entire decimal portion of the number.

Rather than printing things in the functions, you should put those statements in the switch statement. This is more a matter of style than anything else, but I feel that it cleans the functions up a little and makes them easier to reuse. It also keeps them more focussed. Remember that all statements between { } count as one block. You can use these in your switch statement as follows:


switch(letter) {
case 'c':
{
cout << "Enter a celsius temperature . . .";
float celsiusVal;
cin >> celsiusVal;
cout << "In Fahrenheit: ";
cout << getCtemp(celsiusVal);
}
break;

case 'f':
{
cout << "Enter a fahrenheit temperature . . .";
float fahrenheitVal;
cin >> fahrenheitVal;
cout << "In Celsius: ";
cout << getFtemp(fahrenheitVal);
}
break;

default:
cout << "ERROR: You did not enter either \'c\' or \'f\'";
}





You can see here that I've also given your functions parameters. The value is read in and then passed to the function. That way, if you change main at a later date (say to have just one input like 32 F), you don't have to worry about messing around with your functions; you just call the right one with the value.

EDIT: mikeman beat me to it.
EDIT 2: Actually, you don't need to put the statements in { }.

Share this post


Link to post
Share on other sites
Quote:

This is more a matter of style than anything else, but I feel that it cleans the functions up a little and makes them easier to reuse.


Then it's far more than just "style". It's about proper design, having indepedent modules that have a single responsibility(only doing the conversion, not also printing the results) and can be used by any higher-level module(such as main). For example, another module may not want to print the results, but use them in subsequent calculations or displaying them in another fashion such with as with graphics. This is impossible with the OP's design. Of course, he probably doesn't want to do anything else with this little program but that's besides the point; he should learn how to properly do things, after all that's what these little exercises are all about.

Share this post


Link to post
Share on other sites
Quote:
Original post by mikeman
Quote:

This is more a matter of style than anything else, but I feel that it cleans the functions up a little and makes them easier to reuse.


Then it's far more than just "style". It's about proper design, having indepedent modules that have a single responsibility(only doing the conversion, not also printing the results) and can be used by any higher-level module(such as main). For example, another module may not want to print the results, but use them in subsequent calculations or displaying them in another fashion such with as with graphics. This is impossible with the OP's design.


Right. Poor choice of words on my part.

Share this post


Link to post
Share on other sites
Quote:
Original post by ZahlmanDigiDude: Your "correcion" to the arithmetic is quite incorrect.
My bad, you're right, doesn't need correcting. Seems the same though.

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this