If statements acting strangely

Hi all. I've got a program that will (hopefully) run a number of small console-based applications. I'm currently stuck with one of the applications.

The application takes user input, stores it in an array, and retrieves it upon request (sort of like a clipboard with strings only). I also have a method to get a yes/no answer from the user. My error is occurring in a specific call to the getConfirm() method, after I use getline(). The program marks 'y' and 'Y' as invalid input, when they should be valid.

The code for the getConfirm method

Code:

---

bool getConfirm(string q){
        testAgain:
        string s;
        cout<<q<<"[y/n]"<<endl;
        cin>>s;
        if(s == "Y"|| s == "y"){
                return true;
        }
        else if(s == "n"|| s == "N"){
                return false;
        }
        else {cerr<<"Invalid entry"<<endl;
                goto testAgain;
        }
}

---

and for the entire mini-application (It's a self-contained class, initiated by the main method after the user chooses an application)

Code:

---

class Indexer{

        string assignedVals[200];

        public:
                Indexer(){
                        string cont = "new";
                        int val;
                        bool yn;
                        clearVals();
                        begin:
                        while(cont == "new" || cont == "NEW" || cont == "New"){
                                cout<<"Enter an index number below 200 (inclusive) and above 0 (exclusive)"<<endl;
                                cin>>cont;
                                stringstream(cont)>>val;
                                if(val>200){
                                        cout<<"Invalid number: Must be below 200 (inclusive) and not 0(exclusive)"<<endl;
                                        goto begin;
                                }
                                cout<<"Enter the description"<<endl;
                                cin.ignore(100, '\n');
                                getline(cin,cont);
                                assignedVals[val - 1] = cont;
                                cout<<"Select and option: Enter a new value [new], begin retrieval function [ret], or exit (press enter)"<<endl;
                                cin>>cont;
                        }
                        if(cont == ""){
                                exit(0);
                        }
                        else if(cont != "ret" && cont != "Ret" && cont != "RET"){
                                cerr<<"Invalid input"<<endl;
                                bool b = getConfirm("Continue description entry?");//This getConfirm call causes the error.
                                if(b){
                                        goto begin;
                                }
                                else {
                                        b = getConfirm("Start searching?");
                                        if(!b){
                                                exit(0);
                                        }
                                }
                        }
                        do{
                                cout<<"Enter index of value to retrieve"<<endl;
                                cin>>cont;
                                stringstream(cont)>>val;
                                cout<<assignedVals[val-1]<<endl;
                                yn = getConfirm("Get Value Again?");
                        }while(yn);
                        bool b = getConfirm("Begin again?");
                        if(b){
                                clearVals();
                                goto begin;
                        }
                        else exit(0);

                };
        private:
                void clearVals(){
                        for(int i = 0; i<200; i++){
                                assignedVals[0] = "";
                        }
                }
                bool getConfirm(string q){
                        testAgain:
                        string s;
                        cout<<q<<"[y/n]"<<endl;
                        cin>>s;
                        if(s == "Y"|| s == "y"){//Continuously fails only when there is an error
                                return true;
                        }
                        else if(s == "n"|| s == "N"){
                                return false;
                        }
                        else {cerr<<"Invalid entry"<<endl;
                                goto testAgain;
                        }
                }
};

---

If you feel the need to compile and reproduce the error, or need to see output/input, see below:

Code:

---

Enter an index number below 200 (inclusive) and above 0 (exclusive)
1
Enter the description  //Line 3
Hi
Select and option: Enter a new value [new], begin retrieval function [ret], or exit (press enter)
f
Invalid input //This is correct
Continue description entry?[y/n]
y  //This should be valid and return to line 3
Invalid input  //Should not be printed -- should have returned to line 3 already
Continue description entry?[y/n]  //Should not be printed
n  //Should not be printed
Start searching?[y/n]  //Should not be printed
n  //Should not be printed

---

I hope all my terminology and everything else is correct. I'm really a Java developer, so...

All help/questions are welcome,

Singing Boyo

Did you try to run step by step? After cin >> s, what is the value of s?

Code:

---

goto begin;

---

STOP using 'goto'. It is obsolete and shows bad design.

Code:

---

exit(0)

---

Using exit terminates your program on the spot without destructing your objects. Don't use it. Use the normal way of exiting your program.

Quote:

---

Originally Posted by Singing Boyo

Hi all. I've got a program that will (hopefully) run a number of small console-based applications. I'm currently stuck with one of the applications.

---

1) Do not use goto. Doing that in itself renders your program meaningless to many who would want to help you. Anytime you change the flow of code using goto, hardly anyone wants to get involved in following spaghetti logic.

2) std::strings should be passed by reference or const reference if they are not changed, not by value.

3) Please name your variables with meaningful names, not "s" and "q".

Code:

---

#include <string>
#include <iostream>
//...
using namespace std;
//...
bool getConfirm(const string& question)
{
    while (true)
    {
          std::string answer;
          cout << question <<"[y/n]"<<endl;
          cin >> answer;
          if(answer == "Y"|| answer == "y")
              return true;
          else if(answer == "n"|| answer == "N")
              return false;
          else
              cerr<<"Invalid entry"<<endl;
      }
      return false;
}

---

4) Do not use exit(0) in class code. That IMO is a cardinal sin of C++ programming, and that is classes that on purpose, pull the rug out from under you and shuts your program down just because there is an error.

Regards,

Paul McKenzie

Thanks all. I figured out what was happening (didn't initialize the cont string in the right place). I went to look at my goto functions and replaced one of them... But I am wondering what an alternative for exit(0) would be. As far as I can tell it works exactly like return 0 in the main method... If anyone can tell me what would be better, I would be grateful.

Quote:

---

Do not use exit(0) in class code. That IMO is a cardinal sin of C++ programming, and that is classes that on purpose, pull the rug out from under you and shuts your program down just because there is an error.

---

I'm not shutting down because of an error... I'm shutting down because of input AFTER the error, and only if the user decides to shut down.

A class like that shouldn't terminate the app. That's bad design. You should move all the getting input stuff out of the constructor and move it to a separate function, or at least have the constructor set a flag indicating whether the object initialized successfully. Whatever program uses the class can determine whether the state is valid and take whatever action it deems appropriate.

Quote:

---

Originally Posted by Singing Boyo

I'm not shutting down because of an error... I'm shutting down because...

---

That's my point...Never shut down regardless of the reason.

Let the application that is using the class decide what to do when there is an error. You return an error code, or throw an exception if there is an error. What if I had some files open in my app, I use your class, and then you shut the app down on me, not giving me a chance to close the files I had opened?

This sums it up:
http://www.flounder.com/badprogram.htm#ExitProcess

And the best line from that link:

Quote:

---

The arrogance that says a subroutine writer has the right to determine that my program should shut down requires a serious attitude adjustment on the part of that programmer. No subroutine has the right to terminate a program. Only the user has the right to terminate a program.

---

Regards,

Paul McKenzie

Hmm... well, I'm shutting down from user input stating that the program SHOULD be shut down. I present them with the option, and if they choose it, it shuts down. I can't see anything against that...

Quote:

---

Originally Posted by Singing Boyo

Hmm... well, I'm shutting down from user input stating that the program SHOULD be shut down. I present them with the option, and if they choose it, it shuts down. I can't see anything against that...

---

You are still giving the app your options of what to do. You shouldn't be giving any options of what to do.

What if I don't like any of your choices of what to do? You still are not giving the app their own options of what to do when an error occurs. That's why you return an error back to the app, and if it's so bad it should be shut down, throw an exception and let the app handle it.

Any class or subroutine code that has exit(0) in it, regardless of how it's used, is bad.

Regards,

Paul McKenzie