Function won't return varaible from while loop

[GeorgeSDG]

Hi. I am very new to c++ but have some coding experience. As an exercise I tried to make a function that would use an enum variable to return an integer that could be used in a switch. Notwithstanding this a terribly messy way to do it (it was just an exercise), I encountered the following:
Not being able to pass the text input directly into the enum, I made an if loop to do it. This is inside a while(true) loop to make a 'error: try again' part.
What I have found is that the integer variable to be returned works inside the if conditions but reverts to the initialsed value 0 when I break out of the while loop.
The results should be that 'right' returns 0 and 'left' returns 1 but the returned value is always 0; even though in the if condition for 'left' it shows that the variable has been changed to 1 - as soon as the while loop breaks it goes back to 0. Also I have noticed that if I replace 'break;' in the left if-condition with 'return which_way;' does return 1 and the whole thing works properly. So I suppose i could put 'return which_way;' instead of break; in all the if-conditions but that seems like repeating the code (DRY). I have put the full code in below (sorry if that's overkill) and what I get running the program for 'left' look like (my comments with <-):

Initial user way = 0
Initial choice is :
Which way to go: >left

Choice is: left
User choice is passed to function.

In function:
which_way before WHILE : 0
You chose left - in WHILE which_way = 1 <- left returns 1 in if/while loop which I can return to main if I do it specifically
After WHILE which_way = 0 <- after while loop it's back to 0
which_way is returned to main

Your choice is: left, user_way = 0 <- choice

Process returned 0 (0x0) execution time : 4.437 s
Press any key to continue.

TIA - George the confused

  #include <iostream>

// function to return integer from user choice right/left

int user_choice(std::string choice)
{
    enum direction{right, left};
    int which_way = 0;
    std::cout << "In function:\nwhich_way before WHILE : " << which_way <<std:: endl;

    while (true)
    {
        if (choice == "right")
        {
            direction which_way = right;
            std::cout << "You chose right - in WHILE which_way = " << which_way << std::endl;
            break;
        }
        else if (choice == "left")
        {
            direction which_way = left;
            std::cout << "You chose left - in WHILE which_way = " << which_way << std::endl;
            break; //This is where I can put return which_way; and it will work
        }
        else
        {
            std::cout << "Sorry, that is not a valid option. Try again.\n" << std::endl;
        }
    }
    std::cout << "After WHILE which_way = " << which_way << std::endl << "which_way is returned to main\n" << std::endl;
    return which_way;
}

int main()
{
    // get user input (right/left) and pass to function to return integer (0/1)

    int user_way = 0;
    std::string choice = "";

    std::cout << "Initial user way = " << user_way << std::endl;
    std::cout << "Initial choice is : " << choice << std::endl;
    std::cout << "Which way to go: >";

    std::cin >> choice;
    std::cout << "\nChoice is: " << choice << std::endl << "User choice is passed to function.\n" << std::endl;

    user_way = user_choice(choice);

    std::cout << std::endl << "Your choice is: " << choice << ", user_way = "<< user_way << std::endl;

    return 0;
}

[Handy Andy]

Hello GeorgeSDG,

I do not know if you realize that your while loop:

while (true)
{
	if (choice == "right")
	{
		direction which_way = right;
		std::cout << "You chose right - in WHILE which_way = " << which_way << std::endl;
		break;
	}
	else if (choice == "left")
	{
		direction which_way = left;
		std::cout << "You chose left - in WHILE which_way = " << which_way << std::endl;
		break; //This is where I can put return which_way; and it will work
	}
	else
	{
		std::cout << "Sorry, that is not a valid option. Try again.\n" << std::endl;
	}
}

std::cout << "After WHILE which_way = " << which_way << std::endl << "which_way is returned to main\n" << std::endl;

return which_way;

Is an endless loop.
If "choice" equals "right" or "Left" you are OK.

Should you reach the else statement you have an endless loop because the value of "choice" never changes, so there is no way out.

This is untested, but I think what you want is:

else
{
	std::cout << "Sorry, that is not a valid option. Try again.\n" << std::endl;

	std::cout << "Which way to go (right or left):> ";

	std::cin >> choice;
}

Now if you make the correct choice you will break out of the while loop.

Give it a try and see what happens.

Andy

[GeorgeSDG]

Thank you Andy, I hadn’t noticed that. I think in an earlier version the input was in the while loop and the main function called user_choice without passing anything. I brought the input down to main to try and work out my OP problem without realising the consequence. Thanks again and well spotted :)
while loop now starts:

while (true)
    {
        std::cout << "Which way to go: >";
        std::cin >> choice;

        if (choice == "right")

main is now:

int main()
{
    int user_way = 0;
    std::string choice;

    std::cout << "Initial user way = " << user_way << std::endl;
    std::cout << "Initial choice is : " << choice << std::endl;

    user_way = user_choice(choice);

    std::cout << std::endl << "Your choice is: " << choice << ", user_way = "<< user_way << std::endl;

    return 0;
}

Here is a revised output:

Initial user way = 0
Initial choice is :
In function:
which_way before WHILE : 0
Which way to go: >asdasd
Sorry, that is not a valid option. Try again.

Which way to go: >left
You chose left - in WHILE which_way = 1
After WHILE which_way = 0
which_way is returned to main

Your choice is: , user_way = 0

Process returned 0 (0x0)   execution time : 9.966 s
Press any key to continue.

[Handy Andy]

Hello GeorgeSDG,

I did not catch this one until I had the chance to debug the program.

In the function you start with:

int user_choice(std::string choice)
{
	enum direction { right, left };
	int which_way = 0;
	std::cout << "In function:\nwhich_way before WHILE : " << which_way << std::endl;

This is fine.

If you are compiling to the C++11 standards you could write it this way int which_way{};. The empty {}s, the uniform initializer, set the variable to (0) zero.

Then in the while loop you have:

if (choice == "right")
{
	direction which_way = right;
	std::cout << "You chose right - in WHILE which_way = " << which_way << std::endl;
	break;
}

You are redefining the variable "which_way", but since this is inside the while loop it is a local variable to the while and the if statement and over shadows the variable defined outside the while loop.

So by the time you reach the end of the function you are returning the "int which_way" that never changed and the "direction which_way" was destroyed when the if statement and while loop ended. Which means that you are returning the initialized "int which_way" defined at the beginning of the program and that variable has never changed.

When I removed the "direction" it worked correctly.

I changed your prompt a bit see what you think. std::cout << "\n Which way to go (left, right): > "; and for the error message I did:
std::cout << "\n Sorry, that is not a valid option. Try again." << std::endl;

Which gave me the output of:

Initial user way = 0
Initial choice is :
In function:
which_way before WHILE : 0

 Which way to go (left, right): > asdf

    Sorry, that is not a valid option. Try again.

 Which way to go (left, right): > left

You chose left - in WHILE which_way = 1
After WHILE which_way = 1
which_way is returned to main  // <--- Could have said 1 is returned to main.


Your choice is: , user_way = 1

I think the changes you made moving the input and what I found should fix your problems.

Andy

[GeorgeSDG]

Thanks again Andy. And thank you for taking the time to answer a question from a self-confessed noob :)

I did have a suspicion that the problem might have had something to do with the while loop behaving as a function in its own right and treating the variables within it as local; especially because I could return which_way (w_w) from inside the loop and it seemed to work.

I thought that writing "direction w_w = right/left" would simply return w_w as an integer using the specifics in the enum declaration; that is "direction w_w = left;" was equivalent to "w_w = 1;" and that having defined w_w as an integer to begin with that would work.

It's interesting to see that you don't need to preface with "direction" to change the integer value of w_w, you only need "w_w = right/left;" and the program seems to understand to refer to the enum implicitly.

Anyway, onward I go on my c++ journey and thanks very much once again.