4
\$\begingroup\$

Expanding the title: my goal (for hobby project) was to create simple, generic (customizable) and cross-platform menu class for console applications (mostly simple, text-based games). That's what I came up with:

#include <functional>
#include <iostream>
#include <map>
#include <string>

class ConsoleMenu {
  public:
    ConsoleMenu(const std::string& message,
        const std::string& invalidChoiceMessage,
        const std::string& prompt,
        const std::map<std::string, std::function<void()>>& commandsByChoice) :
            message_(message), invalidChoiceMessage_(invalidChoiceMessage),
            prompt_(prompt), commandsByChoice_(commandsByChoice) {}
    void use() {
      std::string userChoice;
      std::cout << '\n';
      // Based on https://isocpp.org/wiki/faq/input-output#istream-and-ignore {
      while ((std::cout << message_) && printChoices() &&
          (std::cout << prompt_) && (!(std::getline(std::cin, userChoice)) ||
          !validChoice(userChoice))) {
        std::cout << '\n' << invalidChoiceMessage_;
        // Should there be calls to std::cin::clear and std::cin::ignore, even
        // if I'm using std::getline?
      }
      // }
      commandsByChoice_[userChoice]();
    }
  private:
    bool printChoices() {
      bool successful = false;
      for (auto commandByChoice : commandsByChoice_) {
        successful = static_cast<bool>(
            std::cout << '[' << commandByChoice.first << ']' << '\n');
      }
      return successful;
    }
    bool validChoice(const std::string& choice) {
      return commandsByChoice_.find(choice) != commandsByChoice_.end();
    }
    std::string message_; // label_?
    std::string invalidChoiceMessage_;
    std::string prompt_;
    std::map<std::string, std::function<void()>> commandsByChoice_;
};

int main() {
  bool running = true;
  do {
    ConsoleMenu menu("What should this program do?\n", "Invalid operation; ", "> ", {
      {"run", [&running] {
        running = true;
      }},
      {"stop", [&running] {
        running = false;
      }}
    });
    menu.use();
  } while (running);
}

As you can see, while the general look of the menu is hard-coded, user can (and have to) specify the message (label) of the menu, list of choices, commands that are keyed to these choices, and even error message, and input prompt. "Generic"? Check!

If someone would like to suggest improvement requiring C++17, please, include also some non-standard equivalent(s) with similar interface, for example from The Boost library.

\$\endgroup\$
4
\$\begingroup\$

I see some things that may help you improve your program.

Separate the interface and implementation

Perhaps you posted it as just one file for the sake of simplicity and compactness, but if you were to actually use this, I think it's more likely that it would be useful as separate header and implementation files. This has the additional benefit of making the interface part of the code (the part that a user would have to see) a little shorter and easier to comprehend.

Allow the user to specify streams

Right now, the code is tied to std::cin and std::cout, but what if I wanted to use the menu, say, remotely over a socket connection? This could very easily be allowed by adding input and output stream references to the class. One way to do it would be to have std::istream and std::ostream arguments with defaults to std::cin and std::cout.

Prefer const references

The printChoices routine is currently this:

bool ConsoleMenu::printChoices() {
  bool successful = false;
  for (auto commandByChoice : commandsByChoice_) {
    successful = static_cast<bool>(
        out << '[' << commandByChoice.first << ']' << '\n');
  }
  return successful;
}

The for loop should instead use const auto & rather than auto to assure no copies are made of the strings. Further, the successful flag isn't really necessary. Instead we could write it like this:

bool ConsoleMenu::printChoices() const {
  for (const auto &commandByChoice : commandsByChoice_) {
    out << '[' << commandByChoice.first << ']' << '\n';
  }
  return !!out;
}

The !!out construct assures that out is interpreted as a bool but you could use a cast if you think it's more readable. (Perhaps I've been writing too much Javascript lately!)

Prefer unordered containers for performance

The std::map container has logarithmic search performance, but the std::unordered_map container has constant time performance. For that reason, it makes sense to prefer the unordered container unless there's actually a need for a sorted one. There isn't in this case.

Use const where practical

Right now, it's not possible to declare and use a const ConsoleMenu. There's no reason that shouldn't be possible. To do that, all of the functions except the constructor would have to be const. Mostly that means simply adding const to the declaration of the function, but we need to do a little more to adapt use. The reason is the last line:

commandsByChoice_[userChoice]();

There are a few problems with this simple line. First, the operator[] has a side effect you probably don't want. Specifically, if the key is not found, it is inserted into the map. That's definitely not what's wanted here whether we are trying for const or not. Two alternatives are using at (which can throw an exception) or find. A way to do that would be this:

const auto result{commandsByChoice_.find(userChoice)};
if (result != commandsByChoice_.end()) {
    result->second();
}

Since you're already calling find in the helper function, it would make sense to only call it once and save (and use) the result. I'll show that later on.

Rename use

The use method is a bit awkwardly named in my view. I'd suggest instead to use operator()(). This would allow us to rewrite the sample main like this:

int main() {
    bool running{true};
    const ConsoleMenu menu{"What should this program do?\n", 
            "Invalid operation; ", "> ", 
        {
            {"run", [&running] { running = true; }},
            {"stop", [&running] { running = false; }}
        }};
    while(running) {
       menu();
    } 
}

Note that I consistently use the C++11 uniform initialization syntax to reduce ambiguity.

Avoid re-doing work

Because there is no way within this code to insert or delete menu choices once the ConsoleMenu is created, we can create the entire menu as a static string once (perhaps within the constructor) and simply use that rather than iterating through the menu choices and making multiple calls to operator<< every time printChoices() is called. Further, one could create a single string that includes message_, all choices and prompt_.

Results

Using all of the suggestions above, we can simplify considerably and remove the two private helper functions. Here's the header file:

menu.h

#ifndef MENU_H
#define MENU_H
#include <functional>
#include <iostream>
#include <unordered_map>
#include <string>

class ConsoleMenu {
  public:
    ConsoleMenu(const std::string& message,
        const std::string& invalidChoiceMessage,
        const std::string& prompt,
        const std::unordered_map<std::string, std::function<void()>>& commandsByChoice,
        std::istream &in = std::cin,
        std::ostream &out = std::cout);
    void operator()() const;
  private:
    std::string invalidChoiceMessage_;
    // the prompt_ string contains the message, all choices and prompt
    std::string prompt_;
    std::unordered_map<std::string, std::function<void()>> commandsByChoice_;
    std::istream &in;
    std::ostream &out;
};
#endif // MENU_H

And this is the implementation.

menu.cpp

#include "menu.h"
#include <sstream>

ConsoleMenu::ConsoleMenu(const std::string& message,
    const std::string& invalidChoiceMessage,
    const std::string& prompt,
    const std::unordered_map<std::string, std::function<void()>>& commandsByChoice,
    std::istream &in, std::ostream& out) :
        invalidChoiceMessage_{invalidChoiceMessage},
        commandsByChoice_{commandsByChoice},
        in{in}, 
        out{out} 
{
    std::stringstream ss;
    ss << message;
    for (const auto &commandByChoice : commandsByChoice_) {
      ss << '[' << commandByChoice.first << ']' << '\n';
    }
    ss << prompt;
    prompt_ = ss.str();
}

void ConsoleMenu::operator()() const {
    std::string userChoice;
    const auto bad{commandsByChoice_.cend()};
    auto result{bad};
    out << '\n';
    while ((out << prompt_) && (!(std::getline(in, userChoice)) ||
            ((result = commandsByChoice_.find(userChoice)) == bad))) {
        out << '\n' << invalidChoiceMessage_;
    }
    result->second();
}

Additional features

There are some additional things that might be nice. One thing that comes immediately to mind is that it would be useful to be able to create a constexpr ConsoleMenu. One way to do that might be to use C++17's std::string_view instead of std::string and to use an array instead of a map.

Another feature that might be nice would be the ability to optionally use wide strings. This is pretty easy to accommodate by turning the class into a templated class.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.