5
\$\begingroup\$

Some time ago, I wrote this answer to a question about creating a command line menu. I referred to it recently and noticed a few things that I wanted to improve.

Purpose

As with the original version, the purpose is to have a class which simplifies the construction and use of a command line (console) menu.

The improvements I made are:

  1. allow either std::string or std::wstring prompts and answers
  2. allow the user to separate selectors from descriptions
  3. move everything into a header-only module
  4. allow creation of const menus
  5. sort by selectors

Questions

Some things I had questions about are:

  1. template parameter names - could they be improved?
  2. use of default_in and default_out - would it be better to infer defaults from the string type?
  3. choice of std::function<void()> as the operation for each choice
  4. use of std::pair vs. custom object
  5. should I wrap all of this in a namespace?
  6. is any functionality missing?
  7. is there a way to make a constexpr version?

menu.h

#ifndef MENU_H
#define MENU_H
#include <functional>
#include <iostream>
#include <map>
#include <string>
#include <utility>

template <typename T> struct default_in;

template<> struct default_in<std::istream> { 
    static std::istream& value() { return std::cin; }
};

template<> struct default_in<std::wistream> { 
    static std::wistream& value() { return std::wcin; }
};

template <typename T> struct default_out;

template<> struct default_out<std::ostream> { 
    static std::ostream& value() { return std::cout; }
};

template<> struct default_out<std::wostream> { 
    static std::wostream& value() { return std::wcout; }
};

template <class str, class intype, class outtype>
class ConsoleMenu {
  public:
    ConsoleMenu(const str& message,
        const str& invalidChoiceMessage,
        const str& prompt,
        const str& delimiter,
        const std::map<str, std::pair<str, std::function<void()>>>& commandsByChoice,
        intype &in = default_in<intype>::value(),
        outtype &out = default_out<outtype>::value());
    void operator()() const;
  private:
    outtype& showPrompt() const;
    str message;
    str invalidChoiceMessage_;
    str prompt;
    str delimiter;
    std::map<str, std::pair<str, std::function<void()>>> commandsByChoice_;
    intype &in;
    outtype &out;
};

template <class str, class intype, class outtype>
ConsoleMenu<str, intype, outtype>::ConsoleMenu(const str& message,
    const str& invalidChoiceMessage,
    const str& prompt,
    const str& delimiter,
    const std::map<str, std::pair<str, std::function<void()>>>& commandsByChoice,
    intype &in, outtype& out) :
        message{message},
        invalidChoiceMessage_{invalidChoiceMessage},
        prompt{prompt},
        delimiter{delimiter},
        commandsByChoice_{commandsByChoice},
        in{in}, 
        out{out} 
{}

template <class str, class intype, class outtype>
outtype& ConsoleMenu<str, intype, outtype>::showPrompt() const {
    out << message;
    for (const auto &commandByChoice : commandsByChoice_) {
      out << commandByChoice.first 
            << delimiter
            << commandByChoice.second.first
      << '\n';
    }
    return out << prompt;
}

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

#endif // MENU_H

main.cpp

#include "menu.h"
#include <iostream>
#include <functional>

template <class str, class outtype>
class Silly {
public:
    void say(str msg) {
        default_out<outtype>::value() << msg << "!\n";
    }
};

using MySilly = Silly<std::string, std::ostream>;

int main() {
    bool running{true};
    MySilly thing;
    auto blabble{std::bind(&MySilly::say, thing, "BLABBLE")};
    const ConsoleMenu<std::string, std::istream, std::ostream> menu{
        "What should the program do?\n",
        "That is not a valid choice.\n",
        "> ",
        ". ",
        {
            { "1", {"bleep", []{ std::cout << "BLEEP!\n"; }}},
            { "2", {"blip", [&thing]{ thing.say("BLIP"); }}},
            { "3", {"blorp", std::bind(&MySilly::say, thing, "BLORP")}},
            { "4", {"blabble", blabble }},
            { "5", {"speak Chinese", []{std::cout << "对不起,我不能那样做\n"; }}},
            { "0", {"quit", [&running]{ running = false; }}},
        }
    };
    while (running) {
        menu();
    }
}

This shows the use of the program and several different ways of creating menu functions. Depending on your console and compiler settings, the Chinese sentence may or may not be displayed properly. Next is a wide string version.

wide.cpp

#include "menu.h"
#include <iostream>
#include <functional>
#include <locale>

template <class str, class outtype>
class Silly {
public:
    void say(str msg) {
        default_out<outtype>::value() << msg << "!\n";
    }
};

using MySilly = Silly<std::wstring, std::wostream>;

int main() {
    bool running{true};
    MySilly thing;
    auto blabble{std::bind(&MySilly::say, thing, L"BLABBLE")};
    ConsoleMenu<std::wstring, std::wistream, std::wostream> menu{
        L"What should the program do?\n",
        L"That is not a valid choice.\n",
        L"> ",
        L". ",
        {
            { L"1", {L"bleep", []{ std::wcout << L"BLEEP!\n"; }}},
            { L"2", {L"blip", [&thing]{ thing.say(L"BLIP"); }}},
            { L"3", {L"blorp", std::bind(&MySilly::say, thing, L"BLORP")}},
            { L"4", {L"blabble", blabble }},
            { L"5", {L"说中文", []{std::wcout << L"对不起,我不能那样做\n"; }}},
            { L"0", {L"quit", [&running]{ running = false; }}},
        }
    };
    std::locale::global(std::locale{"en_US.UTF-8"});
    while (running) {
        menu();
    }
}
\$\endgroup\$
4
  • \$\begingroup\$ Perhaps more event driven and model-view-controller like architecture would be better? Usually there is some shared state that needs to change and there is usually only one screen to render into. Perhaps I’m being biased towards OpenGL though. \$\endgroup\$ Commented Dec 19, 2020 at 18:03
  • \$\begingroup\$ It could have been separated, but there's so little of the view and controller pieces that it didn't seem worthwhile to do that. Also keep in mind that one could easily wrap this inside another class to maintain whatever shared state might be needed. The running variable was intended to illustrate that kind of use. \$\endgroup\$
    – Edward
    Commented Dec 19, 2020 at 18:09
  • \$\begingroup\$ The use cases I had in mind were multi-menu situations and submenu cases. There are also conditions when menu needs to be rebuilt or some parts of it disabled. For example not letting the user exit without explicitly either saving or discarding current state. \$\endgroup\$ Commented Dec 19, 2020 at 18:24
  • \$\begingroup\$ Oh, I see what you mean. I had in mind a simple single-level menu that was small and simple enough to be useable on an embedded system. An answer pointing out the use case you mention and what is lacking in the current code to support it would be welcome! \$\endgroup\$
    – Edward
    Commented Dec 19, 2020 at 18:31

1 Answer 1

3
\$\begingroup\$

Answers to your questions

template parameter names - could they be improved?

Mostly it's that they are inconsistent. Start type names with a capital, and either suffix them all with Type or don't. I suggest:

  • str -> Str
  • intype -> IStream (just to be clear the we do expect something like std::istream here)
  • outtype -> OStream

use of default_in and default_out - would it be better to infer defaults from the string type?

Yes, see below.

choice of std::function<void()> as the operation for each choice

You need std::function<> here to store the functions for each choice in the map. The only question is if void() is the right type for the function. If you wanted operator()() to take parameters and/or return a value, then you would have to change the type of the function as well.

use of std::pair vs. custom object

I personally think it's fine with std::pair.

should I wrap all of this in a namespace?

If it is just class ConsoleMenu, I don't think it would be any improvement to put it in a namespace. However, I would put default_in and default_out in a namespace, as those names are quite generic, and you don't want them to pollute the global namespace.

is any functionality missing?

I don't know, if this is all you need then it's complete. If you need something else from it, it's not.

is there a way to make a constexpr version?

Yes, by making sure it satisfies the requirements of LiteralType. This also means that all member variables must be valid LiteralTypes, and that prevents using std::string or std::map. You can use const char * and std::array instead.

Pass the input and output stream by value

The construction you have where you pass a stream type as a template parameter, and then have it deduce a concrete stream from that is very weird, inflexible, and requires more typing than necessary. Just add the input and output stream as parameters to the constructor:

template <class str, class intype, class outtype>
class ConsoleMenu {
public:
    ConsoleMenu(const str& message,
        ...,
        intype &in,
        outtype &out);

Compare:

ConsoleMenu<std::wstring, std::wistream, std::wostream> menu{...}

Versus:

ConsoleMenu<std::wstring> menu{..., std::wcin, std::wcout}

If you want the standard input and output to be a default parameter, then I would deduce it from the string type:

template <typename T> struct default_in;

template<> struct default_in<std::string> { 
    static std::istream& value() { return std::cin; }
};

template<> struct default_in<std::wstring> { 
    static std::wistream& value() { return std::wcin; }
};

...

template <class str, class intype, class outtype>
class ConsoleMenu {
public:
    ConsoleMenu(const str& message,
        ...,
        intype &in = default_in<str>::value(),
        outtype &out = default_out<str>::value());

Because then you can just write:

ConsoleMenu menu{L"Wide menu", L"invalid", L"> ", L". ", {/* choices */}};
\$\endgroup\$
2
  • \$\begingroup\$ I also noticed another flaw with my version. If I try to use std::string_view, the code won't compile because it tries to read one from std::cin. I think I'll have to key the type off of the string type such that wstring_view becomes wstring. Still pondering how best to do that. \$\endgroup\$
    – Edward
    Commented Dec 20, 2020 at 14:51
  • 1
    \$\begingroup\$ You could use str::value_type perhaps? And then use intype = std::basic_istream<str::value_type> as the default parameter value. \$\endgroup\$
    – G. Sliepen
    Commented Dec 20, 2020 at 16:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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