What am I doing wrong? (C++ problemo)

[Fruitpastles]

I got a book on C++ just before christmas and I've been slowly working my way through it. Yesterday I tried to make a small program to convert fahrenheit to celcius but it isn't working for some reason?

#include <iostream>



using namespace std;





//define vars

int tempVal = 0;

char scale[1];

int conversion = 0;



//function to convert F to C

int toCelsius (int tempVal)

{

 conversion = tempVal - 32 / 9 *5; //formula for conversion



 return conversion;

}





//function to convert C to F

int toFahrenheit (int tempVal)

{

conversion = tempVal + 32 * 9 /5; //formula for conversion

return conversion;

}





int main()

{

  cout<<"Enter the Temperature Value: ";

  cin>> tempVal;

  cout<< "Enter the Temperature Scale: ";

  cin>> scale;

  if (scale == "f")

  {

    cout<< toCelsius(tempVal); //call function to convert to C

  }

  else if (scale == "c")

  {

     cout<< toFahrenheit (tempVal); //call function to convert to F

  }

  else

  {

   cout <<"Error"; // this is what it outputs every time :(

  }



  cin.get();

  return 0;

}

Every time it outputs "Error" ( cout <<"Error"; ) and I'm sure I'm entering it in lower case before anyone asks.

Any help is greatly appreciated . :D

[Goldy]

cout<<"Enter the Temperature Value: ";

cin>> tempVal;

cout<< "Enter the Temperature Scale: ";

cin>> scale;

if (scale == "f")

{

cout<< toCelsius(tempVal); //call function to convert to C

}

else

{

if (scale == "c")

{

cout<< toFahrenheit (tempVal); //call function to convert to F

}

else

{

cout <<"Error"; // this is what it outputs every time :(

}

}

fixed, I think, i'm not a c++ man, only get as far as c# but it does the job

[cooper]

fixed, I think, i'm not a c++ man, only get as far as c# but it does the job

if - else if - else is a valid construct.

The problem is that you're comparing strings using '=='.

Either change the ifs in which you compare scale to something with a strcmp(str1,str2)==0 or compare the first character of the string with your fixed value like so: if (scale[0] == 'f')

(note the single quotes to specify the fact that f is a single character)

Remember that a string is a pointer to an array of characters. When you compare them with == you're effectively comparing the pointer value, which will only match when they're pointing at the exact same memory location.

[jollyrancher82]

fixed, I think, i'm not a c++ man, only get as far as c# but it does the job

if - else if - else is a valid construct.

The problem is that you're comparing strings using '=='.

Either change the ifs in which you compare scale to something with a strcmp(str1,str2)==0 or compare the first character of the string with your fixed value like so: if (scale[0] == 'f')

(note the single quotes to specify the fact that f is a single character)

Remember that a string is a pointer to an array of characters. When you compare them with == you're effectively comparing the pointer value, which will only match when they're pointing at the exact same memory location.

You could also do *scale == 'c' or *scale == 'f', but that's pointers :P

[wetelectric]

Validating your input although not really necessary for this simple exercise, would be a good habit to get into :)

[Fruitpastles]

Thanks a lot everyone it's working now. I used the array method

cooper mentioned. Thanks.

[cooper]

Your next mission, should you choose to accept it, is to turn the if - else if - else construct into a switch statement.

[Oberon]

yeah, switch is cleaner :) saves the if else if else if else if...else

[Fruitpastles]

Thanks for suggesting a switch statement:

switch(scale[0])

 {

        case 'f':

         cout<< "The value in Degrees Celsius is: " << toCelsius(tempVal);

         cin.get();

          break;

        case 'F':

         cout<< "The value in Degrees Celsius is: " << toCelsius(tempVal);

         cin.get();

          break;

        case 'c':

         cout<< "The value in Degrees Fahrenheit is: " << toFahrenheit (tempVal);

         cin.get();

          break;

        case 'C':

        cout<< "The value in Degrees Fahrenheit is: " << toFahrenheit (tempVal);

        cin.get();

          break;

        default:

          cout<< "Error";

          break;

 }

[cooper]

Whenever you see blocks repeated in code, you know you're probably doing something sub-optimal.

switch(scale[0])

 {

        case 'f':

        case 'F':

         cout<< "The value in Degrees Celsius is: " << toCelsius(tempVal);

         cin.get();

          break;

        case 'c':

        case 'C':

        cout<< "The value in Degrees Fahrenheit is: " << toFahrenheit (tempVal);

        cin.get();

          break;

        default:

          cout<< "Error";

          break;

 }

or better still

#include <cctype.h>

[...]

 switch(toupper(scale[0]))

 {

        case 'F':

         cout<< "The value in Degrees Celsius is: " << toCelsius(tempVal);

          break;

        case 'C':

        cout<< "The value in Degrees Fahrenheit is: " << toFahrenheit (tempVal);

          break;

        default:

          cout<< "Error";

          break;

 }

 cin.get();

[Oberon]

Whenever you see blocks repeated in code, you know you're probably doing something sub-optimal.

switch(scale[0])

 {

        case 'f':

        case 'F':

         cout<< "The value in Degrees Celsius is: " << toCelsius(tempVal);

         cin.get();

          break;

        case 'c':

        case 'C':

        cout<< "The value in Degrees Fahrenheit is: " << toFahrenheit (tempVal);

        cin.get();

          break;

        default:

          cout<< "Error";

          break;

 }

or better still

#include <cctype.h>

[...]

 switch(toupper(scale[0]))

 {

        case 'F':

         cout<< "The value in Degrees Celsius is: " << toCelsius(tempVal);

          break;

        case 'C':

        cout<< "The value in Degrees Fahrenheit is: " << toFahrenheit (tempVal);

          break;

        default:

          cout<< "Error";

          break;

 }

 cin.get();

I'd have to say that I like the second one much better. :)

[psychoaliendog]

or even simpler yet

[...]

 switch(scale[0] & 0xdf)

 {

        case 'F':

         cout<< "The value in Degrees Celsius is: " << toCelsius(tempVal);

          break;

        case 'C':

        cout<< "The value in Degrees Fahrenheit is: " << toFahrenheit(tempVal);

           break;

        default:

          cout<< "Error";

          break;

 }

 cin.get();

There is no need to use some fancy "toupper" function when its a simple bit mask, maybe some bounds checking if you don't want to be so messy. It not more than a line of code.

Also whats with the single element array? You shouldn't ever need a single element array, especially when working with strings (Remember strings are null terminated, so, the minimum length of a string should 2). Cin does accept a plain old char variable, that may simplify your code a lot.

Lastly, are those global variables? Bad Programmer, go to your room. Not only that but you are using the same variable names inside the functions. Quick question, do you know which variable you are using in the function? Lets rewrite the program see if we can't clean it up

#include <iostream>



using namespace std;



//function to convert F to C

int toCelsius (int tempVal)

{

  return tempVal - 32 / 9 *5;//formula for conversion

}





//function to convert C to F

int toFahrenheit (int tempVal)

{

  return tempVal + 32 * 9 /5; //formula for conversion

}





int main()

{

  //define vars

  int tempVal = 0;

  char scale;



  cout << "Enter the Temperature Value: ";

  cin >> tempVal;

  cout << "Enter the Temperature Scale: ";

  cin >> scale;



  switch(scale & 0xdf)

  {

        case 'F':

         cout<< "The value in Degrees Celsius is: " << toCelsius(tempVal);

          break;

        case 'C':

        cout<< "The value in Degrees Fahrenheit is: " << toFahrenheit(tempVal);

          break;

        default:

          cout<< "Error";

          break;

  }

  cin.get(); 

  return 0;

}

Oh, one last thing, are you sure that you're properly converting between Celsius and Fahrenheit? You may want to look into the use of parenthesis.

[Fruitpastles]

Thanks Cooper & psychoaliendog.

Thanks for cleaning my code up, but i don't quite understand what "0xdf" does? (I googled it but it came up with nothing, and there wasn't anything about it in ym book) I also put the "cin.get();"s you took out, back in, as without them the console always closes.

I realized my mistake with the formulas yesterday and fixed them using parenthesis thanks for pointing it out anyway.

[cooper]

i don't quite understand what "0xdf" does?

What he's doing is bitwise-and on the character you read with a hexadecimal constant 0xdf. Binary logic and all. There's probably a page or 2 about that in your book, but most tend to glance over it a bit. It's very powerful stuff though.

Now, let's take a look how this bitwise anding thing works:

c = 0x63 = 01100011

C = 0x43 = 00100011

f = 0x66 = 01100110

F = 0x46 = 00100110

Notice that, bitwise, always the 2nd bit from the left is set to zero to make something uppercase.

0xdf = 10111111

When you do bitwise and, you only keep the 1's that are 1 in both values. The 0xdf bitmask ensures that all 1's are copied over unless the second one from the left, effectively giving you the toupper functionality. Readability is a bit less, but performance should be better as you're not performing a function call (though I wouldn't be surprised if toupper was a macro that did just that, so the end result is identical performance-wise).

I also put the "cin.get();"s you took out, back in, as without them the console always closes.

They weren't taken out. They were replaced by one cin.get directly after the switch statement. It should still be called.

[Fruitpastles]

Thanks for explaining, I understand a little more now.

The cin.get(); after the switch statement was already there, but when i tested it it closed after it returned the new value, so I never got to see the new value.

[psychoaliendog]

yup, bit masking is some very cool, dirty, fun. The toupper is probably something along the lines of

#define toupper(x)  ((x >= 'a' && x <= 'z')? x & 0xdf : x)

Which, to the untrained eye, looks more like somebodies keyboard exploded. :D

to clarify the Bitwise and (&) is a truth table.

X & Y = Z

-----------

1 | 1 | 1

0 | 1 | 0

1 | 0 | 0

0 | 0 | 0

so

'c' & 0xdf = 'C'

----------------

0  |    1   |  0

1  |    0   |  0

1  |    1   |  1

0  |    1   |  0

0  |    1   |  0

0  |    1   |  0

1  |    1   |  1

1  |    1   |  1

this is just one of many applications for bitwise operators. If your interested in more check out bitwise or (|), bitwise exclusive or (^), bit shift left (<<) and right (>>).

[Anthrax]

i wrote this exact program on my degree.

heres the code. untested.

//---------------------------------------------------------------------------



#pragma hdrstop

#include <iostream>

#include <conio>

using namespace std;

//---------------------------------------------------------------------------



#pragma argsused

int main(int argc, char* argv[])

{

   float fahrenheit;

   float celsius;



   cout <<"Enter Temperature in Fahrenheit: ";

   cin >>fahrenheit;



   celsius = (fahrenheit + 40) * 5 / 9 - 40;

   cout << endl;

   cout << "Temperature Conversion Table";

   cout << "nnFahrenheit | Celsius";

   cout << "n-----------|--------";

   cout << "n" <<fahrenheit;

   cout << "tt" <<celsius;



   getch ();







   return 0;

}

//---------------------------------------------------------------------------

[ShadowHax]

In case your still confused with hex/octal heres a great resource, not only for numbers but for all of C++

--- The Code Project -- (This is more .NET)

http://www.codeproject.com/

http://www.codeproject.com/cpp/

-- C++ Tutorials/Guides ---

http://www.cplusplus.com/

http://www.cplusplus.com/doc/hex.html