Difficulties with DO, WHILE, IF, ELSE

[miguecor]

Hi to everyone,

I'm very new here and this may be a very dull question, but I've been trying to figure this out for hours and can't get it to work as I want.

I'm 100% new to C++, have been learning for a couple of days, and I was trying to solve an excersise. When I see the code, it makes sense to me (but apparently not to the computer), so could you help me a little?

The problem is that it doesn't matter what value you give to the variable "select", the program is always looping asking for the operation that you want to work with.


Here's the code (I took it from another post in this forum, but wanted to make some changes):

#include<iostream>

using namespace std;

int main()
{
double f, c;
int select;
do
{
    cout << "Please select from the following: " << endl;
    cout << "1) Fahrenheit-to-Celsius" << endl;
    cout << "2) Celsius-to-Fahrenheit" << endl;
    cout << "Select: ";
    cin >> select;
    if(select != 1 || select != 2)
    {
        cout << "Valid options are 1 or 2." << endl;
        system("PAUSE");
        system("CLS");

    }
}
while(select != 1 || select != 2);

if(select == 1 || select == 2)
    {
        if(select == 1)
        {
            cout << " Enter temperature in Fahrenheit to convert to degrees Celsius: ";
            cin >> f;
            c = (f-32) * 5.0 / 9.0;
            cout << "Equivalent in Celsius is: " << c << endl;
        }
        else
        {
            cout <<" Enter temperature in Celsius to convert to degrees Fahrenheit: ";
            cin >> c;
            f = c*9.0/5.0 + 32;
            cout << "Equivalent in Fahrenheit is: " << f << endl;
        }
    }
return 0;
cin.get();
}

Thanks to all.

[Lowest0ne]

At line 24 you say "while select isnt one or select isnt two". Of course, since select can only be one number at a time, one of those conditions will be true.

You want to use && rather than ||. That way you say "while select isnt one and select isnt two"

[Tbonet]

I fixed many things in your code that are considered bad coding practice and just was messy-looking.

1) Don't use "using namespace std;", as this leads to bad coding habits which can cause problems in the future when using two namespaces.
2) NEVER use system, as it is bad coding practice. It is a huge security hole. Try using "#include "conio.h" and "_getch();" in place of "system("PAUSE")".
3) Using switch statements can be a lifesaver, as it cleans up messy if-else statements.
4) "||" means OR while "&&" means AND. You should recognize the difference.
5) Comments are REALLY nice to use. (//)
6) You had some really unnecessary if statements.

#include<iostream>
#include "conio.h"

int main()
{
	double f, c;
	int select;
	
	do{
		std::cout << "Please select from the following: " << std::endl;
		std::cout << "1) Fahrenheit-to-Celsius" << std::endl;
		std::cout << "2) Celsius-to-Fahrenheit" << std::endl;
		std::cout << "Select: ";
		std::cin >> select;
		
		if(select != 1 && select != 2)
		{
			std::cout << "Valid options are 1 or 2." << std::endl << std::endl;
		}
	}while(select != 1 && select != 2);
	
	switch(select)
	{
		case 1:
            std::cout << std::endl  //two endl for space
				<< std::endl << "Enter temperature in Fahrenheit to convert to degrees Celsius: ";
            std::cin >> f;
            c = (f-32) * 5.0 / 9.0;
            std::cout << std::endl << "Equivalent in Celsius is: " << c << std::endl;
			break;
		case 2:
            std::cout << std::endl << std::endl
				<< "Enter temperature in Celsius to convert to degrees Fahrenheit: ";
            std::cin >> c;
            f = c*9.0/5.0 + 32;
            std::cout << "Equivalent in Fahrenheit is: " << f << std::endl;
			break;
      }

	_getch(); 
	return 0;
}

[jkevin]

Never use using namespace std ? why ?

[Tbonet]

1) Don't use "using namespace std;", as this leads to bad coding habits which can cause problems in the future when using two namespaces.

If you program just for yourself, and will never advance into the outside world for programming, then it is fine to use. However, if you actually use MULTIPLE namespaces, it can cause a mix-up.

For example, let's say namespace std has "std::cout", but we use "cout" instead. Another namespace has "another::cout", but if you were to use the namespace, it would be "cout". Notice how they're named the same, but are not the same?

[iamk2]

Another note:

Get rid of the system commands

system("PAUSE");
system("CLS");

and the redundant check for
(select != 1 || select != 2)
and
(select == 1

from here

do
{
    cout << "Please select from the following: " << endl;
    cout << "1) Fahrenheit-to-Celsius" << endl;
    cout << "2) Celsius-to-Fahrenheit" << endl;
    cout << "Select: ";
    cin >> select;
    if(select != 1 || select != 2)
    {
        cout << "Valid options are 1 or 2." << endl;
        system("PAUSE");
        system("CLS");

    }
}
while(select != 1 || select != 2);

just let it scroll - You may like how it clears your screen and yes for this little program on your PC it works. But the overall opinion if not fact is don't get use to using System Commands - for so many reasons you can find through your own search. Clearing a console screen is best done using the ncurses library and you are probably not familiar with that at this point in your studies.

to here

do
{
    cout << "\nPlease select from the following: " << endl;
    cout << "1) Fahrenheit-to-Celsius" << endl;
    cout << "2) Celsius-to-Fahrenheit" << endl;
    cout << "Select: ";
    cin >> select;
}while(select != 1 && select != 2);

also note the moved while

just a suggestion from one newbie to another
my file

#include<iostream>

using namespace std;

int main()
{
double f, c;
int select = 0 ;
while(select != 1 && select != 2)
{
    cout << "\nPlease select from the following: " << endl;
    cout << "1) Fahrenheit-to-Celsius" << endl;
    cout << "2) Celsius-to-Fahrenheit" << endl;
    cout << "Select: ";
    cin >> select;
}
if(select == 1)
    {
        cout << " Enter temperature in Fahrenheit to convert to degrees Celsius: ";
        cin >> f;
        c = (f-32) * 5.0 / 9.0;
        cout << "Equivalent in Celsius is: " << c << endl;
    }
else
    {
        cout <<" Enter temperature in Celsius to convert to degrees Fahrenheit: ";
        cin >> c;
        f = c*9.0/5.0 + 32;
        cout << "Equivalent in Fahrenheit is: " << f << endl;
    }
return 0;
}

do statement gone - and select is initialized

[miguecor]

Thanks to everyone!!!

I still have a couple of doubts:

When I was studying (a few hours ago), I learned that || (OR) evaluates the sentence to the left, and if it's not true, then it evaluates the sentence to the right. I'm still having a little bit of trouble understandig why:

if(select != 1 || select != 2)

was not working :/

No, wait! I think I got it.

@LowestOne / Tbonet / iamk2 : Thanks for the clarifications. I will work harder to type cleaner codes and will avoid use of shortcuts.


@Tbonet
The "switch" option that you showed to me seems to fit perfectly for my first codes in C++



I wonder if _getch() can be used at any point within the code. I'll try it.


I'm not a native english speaker, so maybe I don't find the words to express how grateful I am for your help and for your time: THANKS!


One last question: why some libraries use de ".h" and some others work without it?

[miguecor]

Another last question :P

Is there any significant difference between std::endl and \n ?????

I know that basically either of those do the same, so, why having two operators doing the same?


And what about "goto"??? Is it a good practice to use it????

[iamk2]

the only real difference after compile is that endl will flush your output buffer and \n will not.

flushing this buffer when you don't need that data outputted immediately is better on performance

exp if you have ten or twelve cout statements in a row use the endl on the last and use \n on the first 11.