Jump to content

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


Fruitpastles
 Share

Recommended Posts

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

Link to comment
Share on other sites

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

Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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

Link to comment
Share on other sites

Thanks for suggesting a switch statement:

switch(scale[0])

 {

        case 'f':

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

         cin.get();

          break;

        case 'F':

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

         cin.get();

          break;

        case 'c':

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

         cin.get();

          break;

        case 'C':

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

        cin.get();

          break;

        default:

          cout&lt;&lt; "Error";

          break;

 }

Link to comment
Share on other sites

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

switch(scale[0])

 {

        case 'f':

        case 'F':

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

         cin.get();

          break;

        case 'c':

        case 'C':

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

        cin.get();

          break;

        default:

          cout&lt;&lt; "Error";

          break;

 }

or better still

#include &lt;cctype.h&gt;

[...]

 switch(toupper(scale[0]))

 {

        case 'F':

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

          break;

        case 'C':

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

          break;

        default:

          cout&lt;&lt; "Error";

          break;

 }

 cin.get();

Link to comment
Share on other sites

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

switch(scale[0])

 {

        case 'f':

        case 'F':

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

         cin.get();

          break;

        case 'c':

        case 'C':

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

        cin.get();

          break;

        default:

          cout&lt;&lt; "Error";

          break;

 }

or better still

#include &lt;cctype.h&gt;

[...]

 switch(toupper(scale[0]))

 {

        case 'F':

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

          break;

        case 'C':

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

          break;

        default:

          cout&lt;&lt; "Error";

          break;

 }

 cin.get();

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

Link to comment
Share on other sites

or even simpler yet

[...]

 switch(scale[0] &amp; 0xdf)

 {

        case 'F':

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

          break;

        case 'C':

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

           break;

        default:

          cout&lt;&lt; "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 &lt;iostream&gt;



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 &lt;&lt; "Enter the Temperature Value: ";

  cin &gt;&gt; tempVal;

  cout &lt;&lt; "Enter the Temperature Scale: ";

  cin &gt;&gt; scale;



  switch(scale &amp; 0xdf)

  {

        case 'F':

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

          break;

        case 'C':

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

          break;

        default:

          cout&lt;&lt; "Error";

          break;

  }

  cin.get(); 

  return 0;

}

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

Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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.

Link to comment
Share on other sites

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

#define toupper(x)  ((x &gt;= 'a' &amp;&amp; x &lt;= 'z')? x &amp; 0xdf : x)

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

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

X &amp; Y = Z

-----------

1 | 1 | 1

0 | 1 | 0

1 | 0 | 0

0 | 0 | 0

so

'c' &amp; 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 (>>).

Link to comment
Share on other sites

i wrote this exact program on my degree.

heres the code. untested.

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



#pragma hdrstop

#include &lt;iostream&gt;

#include &lt;conio&gt;

using namespace std;

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



#pragma argsused

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

{

   float fahrenheit;

   float celsius;



   cout &lt;&lt;"Enter Temperature in Fahrenheit: ";

   cin &gt;&gt;fahrenheit;



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

   cout &lt;&lt; endl;

   cout &lt;&lt; "Temperature Conversion Table";

   cout &lt;&lt; "nnFahrenheit | Celsius";

   cout &lt;&lt; "n-----------|--------";

   cout &lt;&lt; "n" &lt;&lt;fahrenheit;

   cout &lt;&lt; "tt" &lt;&lt;celsius;



   getch ();







   return 0;

}

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

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

 Share

  • Recently Browsing   0 members

    • No registered users viewing this page.
×
×
  • Create New...