How to use (or not to use?) goto in a else - if

Question

Every time I use if with goto to bypass a process and to go directly to quit, it doesn't bypass something with a command but it just goes to the end. Here is an example:

#include <iostream>
#include <string>
using namespace std;
int main()
{
  string k;
  cout <<"What would you like to do ?\n"<<"Continue or Exit\n";
  cin >> k;
  if (k = "exit","Exit") goto s;
  else (k = "Continue","continue");
  cout << "You continued...";
  s:
  return 0
}

The problem is that even if you type "exit" or "continue" it just simply closes. Can someone see the problem? Also, how could I avoid using the goto?

Answer 1

The first if is always true, since after the assignment, the test on the RHS of the comma operator will be true.

Your simple program doesn't warrant the use of goto. Also overkill would be my preference for handling such tasks, though: a function table.

I finished off an example. We start with a simple callback class. The callback interface comes with a default implementation that complains about the input.

struct Callback {
    virtual ~Callback () {}
    virtual void operator () (std::string k) const {
        std::cout << "Unrecognized input: " << k << '\n';
    }
};

This part defines the function table data structure, declares an API to initialize one, and the driver code that invokes the callback:

typedef std::map<std::string, const Callback *> CallbackMap;

static void init_table (CallbackMap &cbmap);

static void invoke_table (const CallbackMap &cbmap, std::string k)
{
    CallbackMap::const_iterator i = cbmap.find(k);
    if (i != cbmap.end()) (*i->second)(k);
    else Callback()(k);
}

int main ()
{
    CallbackMap cbmap;
    std::string k;
    init_table(cbmap);
    std::cout << "What would you like to do?\nContinue or Exit\n";
    std::cin >> k;
    invoke_table(cbmap, k);
    return 0;
}

The individual callback implementations are implemented as singletons with this helper macro:

#define CONST_SINGLETON(Name) \
    public: \
        static const Name & instance () { \
            static const Name one; \
            return one; \
        } \
    private: \
        Name () {} \
        Name (const Name &); \
        void operator = (const Name &)

And each callback implementation does the simple action that was presented in your sample program:

class ExitCallback : public Callback {
    void operator () (std::string) const { std::exit(0); }
    CONST_SINGLETON(ExitCallback);
};

class ContinueCallback : public Callback {
    void operator () (std::string) const {
        std::cout << "You continued...\n";
    }
    CONST_SINGLETON(ContinueCallback);
};

The initialization routine populates the function table with the string that corresponds to the appropriate callback:

static void init_table (CallbackMap &cbmap)
{
    static struct { const char *s; const Callback *cb; } init[] = {
        { "exit", &ExitCallback::instance() },
        { "Exit", &ExitCallback::instance() },
        { "continue", &ContinueCallback::instance() },
        { "Continue", &ContinueCallback::instance() },
        { 0, 0 }
    };
    for (int i = 0; init[i].s; ++i) {
        cbmap[init[i].s] = init[i].cb;
    }
};

Answer 2

1) Don't use goto here : it is almost never appropriate to use it in modern C++ code.

2) Use == to compare strings (= is used for string assignments )

 if (k == "exit","Exit") 
      ^^^^
    //Here

 else (k == "Continue","continue")
      ^^^^
   // And here

3) , is not an "and" operator, use :

if (k == "exit" || k == "Exit")

4) You have enclosing braces that are useless, indent your code to properly see which block is really needed.

Answer 3

You need to learn how to use the debugger and you need to learn more about C++

if (k = "exit","Exit") goto s;   //VERY WRONG

does not mean what you want:

• the = of k="exit" is understood as an assignment
• the comma is understood as the comma operator

At least code if (k == "exit" || k == "Exit") and avoid goto when possible.

Also, enable all warnings (and debug info). Perhaps your compiler could warn you.

I would suggest you to read some source code from free software (e.g. on sourceforge or elsewhere). That will teach you a lot.

Answer 4

Proper indenting and style will help you see some of your efforts.

First, I highly recommend transforming the string to either lower or upper case. This eliminates the need to check all permutations of upper and lower case letters in the string:

  cout << "What would you like to do ?\n"
       << "Continue or Exit\n";
  cin >> k;
  std::transform(k.begin(), k.end(), k.begin(), tolower);

To exit the main function, return a value, such as EXIT_SUCCESS or EXIT_FAILURE.

  if (k == "exit")
  {
    return EXIT_SUCCESS;
  } // No goto's necessary
  // The code continues here.

Remember '=' is assignment, '==' is comparison.

There is no need for comparing for "continue" because it does that "automatically" if the response is not "exit".

Using goto
Usage of goto in this instance is not recommended.
If you really must use goto, you will need more than one label.

int main(void)
{
  string k;

top_of_loop:  // Label for your goto
  cout << "What would you like to do ?\n"
       <<"Continue or Exit\n";
  cin >> k;
  std::transform(k.begin(), k.end(), k.begin(), tolower);

  if (k == "exit")
  {
    goto end_of_program;
  }
  // Execution continues here automatically.

  else
  {
    if (k == "continue")
    {
       goto program_continuation;
    }
    else
    {
       cout << "Unknown response, try again.\n";
       goto top_of_loop;
    }
  }
program_continuation:
  cout << "You continued...";
  goto top_of_loop;

end_of_program:
  return 0
}

As a while loop

  cout << "What would you like to do ?\n"
       <<"Continue or Exit\n";
  cin >> k;
  std::transform(k.begin(), k.end(), k.begin(), tolower);
  while (k != "exit")
  {
    // Your program stuff here.
    cout << "Program continued.\n";

    // Prompt the User again.
    cout << "\n"
         << "What would you like to do ?\n"
         << "Continue or Exit\n";
    cin >> k;
    std::transform(k.begin(), k.end(), k.begin(), tolower);
  }
  return 0; // Exit main().

Answer 5

Your code reflects your intentions, but those intentions are not written in C++. They are a valid C++ program - it compiles, of course - but what you meant isn't what you've written.

Here it is, fixed to do what you wished to do.

int main()
{
  string k;
  cout << "What would you like to do ?\nContinue or Exit" << endl;
  cin >> k;
  if (! (k == "exit" || k == "Exit")) {
    if (k == "Continue" || k == "continue") {
      cout << "You continued...";
    }
  }
  return 0
}

You could also do:

int main()
{
  string k;
  cout << "What would you like to do ?\nContinue or Exit" << endl;
  cin >> k;
  if (k == "exit" or k == "Exit") return 0;
  if (k == "Continue" or k == "continue") {
    cout << "You continued...";
  }
  return 0
}

The changes are as follows:

1. To check if one of a multitude of conditions are true, use the logical or operator to join the tests. The operator is written as || or, simply enough, or.

2. The comparison operator is ==. The operator you've used, =, is an assignment operator. It performs the assignment, its resulting value is the assigned value.

3. The comma operator separates multiple expression to be evaluated. Its resulting value is the last expression, even though all expressions are evaluated. For example, the value of (1, 2, 3) is 3.