cannot exit do...while loop

[kuriolis]

Hello all,

I'm a second year computer science student, and before I say anything, this question is regarding an assignment, but this is NOT the solution for the assignment (the assignment's a little more complex than this one function:) ).

My issue is that I'm using a do...while loop in this function, and for some reason, the loop executes endlessly.

Before any long explanations, here's my code. I should mention that the userSaysYes() function is defined in a pre-compiled library given to us by the prof. I'll post the documentation on it as well, so you know what you're looking at. First, the code:

void computeFractions()
{
    Fraction frac1;
    Fraction frac2;
    char userChoice;
    bool yesOrNo;

    do
    {
        cout << "Enter fractional expression to be evaluated: ";
        cin >> frac1 >> userChoice >> frac2;
        if((userChoice == '+')||(userChoice == '-')
            ||(userChoice == '*')||(userChoice == '/'))
        {
            switch(userChoice)
            {
            case '+':
                cout << "\nAnswer: " << frac1 << " + " << frac2 << " = " 
                     << (frac1 + frac2) << endl;
                break;
            case '-':
                cout << "\nAnswer: " << frac1 << " - " << frac2 << " = " 
                     << (frac1 - frac2) << endl;
                break;
            case '*':
                cout << "\nAnswer: " << frac1 << " * " << frac2 << " = " 
                     << (frac1 * frac2) << endl;
                break;
            case '/':
                cout << "\nAnswer: " << frac1 << " / " << frac2 << " = " 
                     << (frac1 / frac2) << endl;
                break;
            }
            yesOrNo = userSaysYes("Do another calculation? "); // <--The problem
        }                                                      
        else                                                  
            exit(EXIT_FAILURE);
    }
    while(yesOrNo == true);  // <--And here also
}                            

And here's the documentation for userSaysYes() :

bool userSaysYes
(
    const string& question //in
);
/**<
Gets the user's yes-or-no answer to a single question.
@return true if the user answers "Yes" to the question asked,
and false otherwise.
@param question Text of the question the user is asked, including the
question mark.
@pre Input stream is clear.
@pre question has been initialized.
@post The text in question has been displayed, followed by
y/[n] and the user has responded by entering a character
and pressing Enter. The value true is returned if the user
responds positively to the question asked by entering a y or
a Y. Entering any other letter or word, as well as simply
pressing the Enter key, will be interpreted as a negative response
and the return value will be false.
*/

(Sorry for how verbose that is, but that's from the header file)

Anyway, here's the output from this function when it executes, along with user input:

Enter fractional expression to be evaluated: 2/3 - 3/4
Answer: 2/3 - 3/4 = -1/12
Do another calculation?  (y/[n]) n
Enter fractional expression to be evaluated:

So as you see, there appears to be a problem with setting the bool "yesOrNo" to the returned value from the bool function userSaysYes(). I have played around with this several different ways, but still have the exact same problem. There's got to be something elementary here that I'm not seeing (or just don't know). If you guys can see whatever it is I'm missing, I'll really appreciate it! :)

[vlad from moscow]

I have not understood about function userSaysYes but it is obvious that its definition you showed is incorrect.

[Moeljbcp]

My first guess would be that the actual function userSaysYes() itself is what's causing the problem. Just listening to the way you describe your problem, it is probably always returning 'true' regardless of whatever input it reads.
I'd start by outputting the result of that function so that you can check whether it's actually returning true or false, and then step through that function manually in the debugger to see if it's working as intended (assuming you're using VC++ or similar). Otherwise just put a bunch of cout debug style statements inside the userSaysYes() function and try to figure out where it's going wrong.

edit: Or, if it's a class supplied library, then I'd ask the professor about it if it doesn't appear to be returning the correct values. I don't see any particular logical errors in your code.

[kuriolis]

Thanks for your observations guys.

I should've mentioned that userSaysYes() and the other functions in that utilities library were given to us by the professor, and not as source code: it's a .obj file.
Also, we've used this function in a couple other assignments, and though not used like this, it worked. By this point, I know that if there was a problem with it, we all would have heard about it.

But if you guys don't see anything obvious wrong with the code, I don't know what to think.

printingvout the value is a good idea though, should have thought of it. :)

[Moeljbcp]

The only other thing I could think of is a problem with the input stream not being completely clear. On line 11 you use cin to get input and don't clear out the leftover "\n" on the stream after you're done. I can't honestly imagine what having a \n leftover in there would do to make the function not work but hey, why not. Try throwing in a cin.ignore() after you get your input and see if that makes any difference whatsoever. The only reason I mention this is because in the documentation for the function it reads "@pre Input stream is clear."

E: Also you could tidy up that switch by placing the part of the cout that reads "Answer: frac1 userChoice frac2 = " outside of the switch and only outputting the answer and an endl inside of it.

[kuriolis]

Well, I tried putting a cin.ignore() after the values are read in to the variables. Good observation, but no difference, unfortunately.

I have tried outputting the value of yesOrNo to the screen, and it is always 0. I've tried initialising the value to false or 1 just for kicks, but by the end of the loop the value is 0 again somehow.

I was thinking and hoping the answer to this would be really simple and obvious, as little things usually are. but if no one else picks up on it, I don't know what to think.

I have to use (or am supposed to) this same loop structure along with userSaysYes() in several other functions in this program, so really wanna get this fixed.

Will be great if my prof responds to my email about this, otherwise I suppose I could just make my own yes/no function. At least it'd probably work. :-)

[Chervil]

did you just do cin.ignore(), or did you try this:
std::cin.ignore( std::numeric_limits<std::streamsize>::max(), '\n' );

[kuriolis]

I did originally use just cin.ignore(), as I'm used to cin.ignore(int, '\n') causing the program to pause until enter is pressed, which is not the behavior I want.

I did just try cin.ignore(80, '\n') to see if it changed anything, but I still have the same problem. :(

[useless]

Change line 11 to:
cin >> frac1 >> ws >> userChoice >> frac2;

[kuriolis]

@useless: Tried, no change.

[DSLq5Di1]

Not related to the problem.. but your if statement is redundant, use case default: in your switch to handle invalid input.

Have you tried isolating userSaysYes() and testing at various stages of your program?

-edit-

kuriolis wrote:

I have tried outputting the value of yesOrNo to the screen, and it is always 0. I've tried initialising the value to false or 1 just for kicks, but by the end of the loop the value is 0 again somehow.

Something fishy going on, if the value is always 0 then while(yesOrNo == true); is false..?

[Moeljbcp]

In case it really is that function that's at fault here's a very simple reimplementation that you can toy with and see if it works:

bool userSaysYes_reimplementation(const string& question)
{
	char choice;
	cout << question << " (y/[n]) ";
	choice = cin.get();
	cin.ignore(INT_MAX, '\n');
	if (choice == 'y' || choice == 'Y')
		return true;
	else
		return false;
}

Let me know if you ever figure out what's wrong, I'm pretty curious now.

sloppy9 wrote:

Something fishy going on, if the value is always 0 then while(yesOrNo == true); is false..?

You know, you're right. It might be something outside the scope of the code he pasted causing it to loop repeatedly. See if you can figure out anything that might be causing that.

[kuriolis]

Again, thanks everyone for the input. :)

@sloppy9: Thanks for the advice, will definitely remove the if...else surrounding that loop.

@Moeljbcp: Thanks for that code. I was going to do that myself when I got home today, but I'll definitely fiddle around with that and see what I can do.

At this point, I'm really curious too. I can't imagine what in the main function could be causing this. The function userSaysYes() is only used in other functions called by main(). BUT, at this point, I'm just going to copy the whole of my main function, along with my includes, the works, just to make sure something doesn't get overlooked.
I'll mention again that the package utilities.h isn't source code, it's an .obj file, so I can only guess what's going on under the hood.

Anyway, here it is:

#include <iostream>
#include <cstdlib>
using namespace std;

#include "fraction.h"
#include "utilities.h"
using Scobey::DisplayOpeningScreen;
using Scobey::Menu;
using Scobey::Pause;
using Scobey::TextItems;
using Scobey::userSaysYes;

/*
do...while loops in compute and compare just keep executing.
Add opening screen to main().
Fully implement option 5.
*/

const TextItems TEXT("compute_with_fractions.txt");

// prototypes
Menu createMenu(Menu& menu);
void computeFractions();
void compareFractions();
void test();

int main()
{

    int userChoice;
    Menu fractionsMenu("Menu");
    createMenu(fractionsMenu);
    fractionsMenu.display();
    userChoice = fractionsMenu.getChoice();
    do
    {
        switch(userChoice)
        {
        case -1:
            return -1;
            break;
        case 1:
            return 0;
            break;
        case 2:
            TEXT.displayItem("Program Description");
            break;
        case 3:
            computeFractions(); // <-- The function in question!
            break;
        case 4:
            compareFractions(); // <-- Also has the same problem
            break;
        case 5:
            test();         // <-- Also uses same structure. Not complete.
            break;
        }
    }
    while(userChoice != 1);
}

Again, bear in mind that this is not completed code, but everything we need to look at is there.

So, if anything jumps out at you guys from that, here's hoping for a quick fix.
Also, if you really need to know anything about those functions from namespace Scobey I can give the documentation, but I don't think that should be an issue.

In the meantime, I'm going to keep messing around, and see what I can come up with.

[brandonator]

YesOrNo is always true because you never gave it a requirement to change that

do something similar to this, although what i did may have created an error or 2 this may help fix your problem

void computeFractions()
{
    Fraction frac1;
    Fraction frac2;
    char userChoice;
    bool yesOrNo;

    do
    {
        cout << "Enter fractional expression to be evaluated: ";
        cin >> frac1 >> userChoice >> frac2;
        if((userChoice == '+')||(userChoice == '-')
            ||(userChoice == '*')||(userChoice == '/'))
        {
            switch(userChoice)
            {
            case '+':
                cout << "\nAnswer: " << frac1 << " + " << frac2 << " = " 
                     << (frac1 + frac2) << endl;
                break;
            case '-':
                cout << "\nAnswer: " << frac1 << " - " << frac2 << " = " 
                     << (frac1 - frac2) << endl;
                break;
            case '*':
                cout << "\nAnswer: " << frac1 << " * " << frac2 << " = " 
                     << (frac1 * frac2) << endl;
                break;
            case '/':
                cout << "\nAnswer: " << frac1 << " / " << frac2 << " = " 
                     << (frac1 / frac2) << endl;
                break;
            }
         if  yesOrNo != userSaysYes
         {
         yesOrNo == false
         }                                                      
    }
    while(yesOrNo == true);  // <--And here also
}                           

[DSLq5Di1]

You are stuck in the outer loop because userChoice never changes. Move the menu display()/getChoice() code into the main loop.

[kuriolis]

ZOMG.

sloppy9, you... are my hero.

Popped that inside the main loop, and that fixed everything.

Honestly, if I hadn't caught that, it might have cost me as much as 6 points out of 30 on my assignment. So sloppy9, by rights I should totally like... I dunno, owe you a pizza or something.

Of course, I can't actually do that, but you know what I mean. :P

And again, thanks everyone for your suggestions. I managed to make my code a bit nicer because of it, and learned a thing or two as well.

Good stuff, guys :)