current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

This code fragment from 2009:

#include "../asklib.hxx"
std::vector<AskBase*> questions;
AskUI<std::vector<AskBase*> > ui(questions);
ui.add(new Ask<std::string>("Enter your name: ", 3, 25));
ui.add(new Ask<std::string>("Enter your city: ", 2, 25));
ui.add(new Ask<std::string>("Enter your state: ", 2, 2));
ui.add(new Ask<int>("Enter your age: ", 18, 150));

was modified to take advantage of C++14 features in the initialization of questions:

#include "../asklib.hxx"
// Define user interface.
std::vector<AskBase*> questions {
   new Ask<std::string>{"Enter your name: ",  3,  25},
   new Ask<std::string>{"Enter your city: ",  2,  25},
   new Ask<std::string>{"Enter your state: ", 2,   2},
   new Ask<int>        {"Enter your age: ",  18, 120}};
AskUI<std::vector<AskBase*>> ui {questions};

int main(){
  ui.run(); 
  if (ui.completed())
    for (auto ii: ui.items())
      std::cout << ii->id() << ii->l_answer_string() << std::endl;
}

As questions is not directly used in the routine later it seems like there should be a way to eliminate the variable by just passing the std::vector<AskBase*> directly the AskUI constructor. Are there suggestions for removing it or rational as to the necessity of it staying and any other comments? I'm looking for C++14 and/or C++1z answers.

The code review aspect is requested for the main file specifically for making the library easier to use. The contents of asklib.hxx and asklib.cxx follow thought other than regarding the improvements needed to support the user of the library it is not presented for the review request.

// asklib.hxx
#ifndef ASKLIB_HXX
#define ASKLIB_HXX
#include <vector>
#include <libintl.h>
#include <locale.h>
#include <string>
#include <iostream>
#include <sstream>
#include <boost/format.hpp>
#define gettext_noop(S) S
// ============================================================================
class AskBase {
// ============================================================================
private:
    class Locale{
    private:
        std::string     m_gtlocale; 
        std::string     m_gtdomain;
        std::string     m_gtdir;
    public:
        Locale(const char* a_gtlocale="", const char* a_gtdomain="asklib", const char* a_gtdir="./exp/po")
            :m_gtlocale(a_gtlocale), m_gtdomain(a_gtdomain), m_gtdir(a_gtdir){setLanguage("pl_PL.utf8");}
        // ----------------------------------------------------------------------------
        void setLanguage(std::string a_language)
        // ----------------------------------------------------------------------------
        {
            m_gtlocale = a_language;
            setlocale(LC_ALL, m_gtlocale.c_str());
            bindtextdomain(m_gtdomain.c_str(), m_gtdir.c_str());
            textdomain(m_gtdomain.c_str());
        }
    };
    struct Status {
        static auto const   c_ROOT          =-999;
        static auto const   c_PREVIOUS      =  -1;
        static auto const   c_NEXT          =   1;
        static auto const   c_EOF           = 999;
    };
    static int              c_instance;
    static const char*      c_e_REQ_EOF;
    static const char*      c_e_REQ_ROOT;
    static const char*      c_e_REQ_PREVIOUS;
    static const char*      c_e_REQ_HELP;
    static const char*      c_e_REQ_VERSION;
    static const char*      c_e_REQ_LANGUAGE;
    static const char*      c_e_MSG_HELP;
    static auto const       c_MAX_TRIES     = 2;
private:
    decltype(AskBase::Status::c_ROOT)   ask_user();
    template<typename C>friend class AskUI;
    virtual bool        validate(std::string a_l_response) = 0;
    int                 m_instance_id;
protected:
    std::string         m_e_raw_prompt;
    std::string         m_l_answer_string;
public:
    Locale              c_locale;
                        AskBase(const char* a_e_prompt);
    void                changeLaguage(std::string a_language){c_locale.setLanguage(a_language);}
    std::string         l_prompt(){return gettext(m_e_raw_prompt.c_str());}
    std::string         l_answer_string(){return m_l_answer_string;}
    auto                id(){return m_instance_id;}
};
// ============================================================================
template<typename T>                            // ex: int, std::string
class Ask : public AskBase{
// ============================================================================
private:
    int     m_min;
    int     m_max;
    bool    validate(std::string a_l_response);
public:
            Ask(const char* a_e_raw_prompt, int a_min, int a_max)
                :AskBase{a_e_raw_prompt}, m_min{a_min}, m_max{a_max}{}
};
// ============================================================================
template<typename Container>
class AskUI {
// ============================================================================
private:
    int status = AskBase::Status::c_ROOT;
public:
    Container&  m_asks;
                AskUI(Container& a_asks, const char* a_gtlocale="")
                    :m_asks{a_asks}{}
    void        add(AskBase* a_ask){m_asks.push_back(a_ask);}
    bool        completed(){return AskBase::Status::c_NEXT==status;};
    Container&  items(){return m_asks;};
    // ----------------------------------------------------------------------------
    auto run()
    // ----------------------------------------------------------------------------
    {
        for (typename Container::iterator ii=m_asks.begin();
                ii!=m_asks.end();
                ii=(AskBase::Status::c_ROOT==status)? m_asks.begin():
                    (AskBase::Status::c_PREVIOUS==status)?(m_asks.begin()==ii?m_asks.begin():ii-1):
                    (AskBase::Status::c_NEXT==status)?ii+1:
                    m_asks.end())
            status = (*ii)->ask_user();
        return;
    }
};
#endif

// asklib.cxx
#include "asklib.hxx"
// Input string the user enters to request program to exit.
const char* AskBase::c_e_REQ_EOF = gettext_noop("^Z");
// Input string the user enters to request control to go to the first prompt in the input tree.
const char* AskBase::c_e_REQ_ROOT = gettext_noop("^^");
// Input string the user enters to request control to go to the previous prompt.
const char* AskBase::c_e_REQ_PREVIOUS = gettext_noop("^");
// Input string the user enters to request version info to be displayed.
const char* AskBase::c_e_REQ_VERSION = gettext_noop("?v");
// Input string the user enters to request a new language. 
const char* AskBase::c_e_REQ_LANGUAGE = gettext_noop("^:");
// Input string the user enters to request help info to be displayed. 
const char* AskBase::c_e_REQ_HELP = gettext_noop("?");
// Help displays useful info for user, Version displays version of software, the remainder are navigation controls: Root
// directs control to the first prompt in the input tree, Previous directs control goes to previous input prompt, Exit
// terminates the program. 
const char* AskBase::c_e_MSG_HELP = gettext_noop(" %1$2s - Help\n %2$2s - Version\n %3$2s - Language\n %4$2s - Root\n %5$2s - Previous\n %6$2s - Exit\n");
int AskBase::c_instance = 0;
// ----------------------------------------------------------------------------
AskBase::AskBase(const char* a_e_prompt)
    :m_e_raw_prompt(a_e_prompt), m_instance_id(++c_instance)
// ----------------------------------------------------------------------------
{
    if (1 == c_instance){
        c_locale.setLanguage("");
        }
}
// ----------------------------------------------------------------------------
decltype(AskBase::Status::c_NEXT) AskBase::ask_user()
// ----------------------------------------------------------------------------
{
    for (auto tries_left = c_MAX_TRIES; tries_left > 0;)
    {
        std::cout << gettext(m_e_raw_prompt.c_str());
        std::string l_response;
        getline(std::cin, l_response);
        if (gettext(c_e_REQ_EOF) == l_response || std::cin.eof() || std::cin.bad())
            return Status::c_EOF;
        else if (gettext(c_e_REQ_ROOT) == l_response)
            return Status::c_ROOT;
        else if (gettext(c_e_REQ_PREVIOUS) == l_response)
            return Status::c_PREVIOUS;
        else if (gettext(c_e_REQ_VERSION) == l_response)
            std::cout << SSVID_ICON << SSVID << std::endl;
        else if (gettext(c_e_REQ_LANGUAGE) == l_response){
            Ask<std::string> lang{"Enter new language: ", 2, 10};
            lang.ask_user();
            std::cout << "New language is: " << lang.l_answer_string() << std::endl;
            c_locale.setLanguage(lang.l_answer_string());
            }
        else if (gettext(c_e_REQ_HELP) == l_response)
            std::cout << boost::format(gettext(c_e_MSG_HELP))
                % gettext(c_e_REQ_HELP) % gettext(c_e_REQ_VERSION) % gettext(c_e_REQ_LANGUAGE) % gettext(c_e_REQ_ROOT) % gettext(c_e_REQ_PREVIOUS) % gettext(c_e_REQ_EOF);
        else if (validate(l_response)){
            m_l_answer_string = l_response;
            return Status::c_NEXT;
        }
        else
            --tries_left;
    }
    return Status::c_EOF;
}
// ----------------------------------------------------------------------------
template<>
bool Ask<std::string>::validate(std::string a_l_response)
// ----------------------------------------------------------------------------
{
    if (a_l_response.length() < m_min)
        std::cout
            << boost::format(gettext("Invalid input. "))
            << boost::format(ngettext("Enter a string at least %d character long. ", "Enter a string at least %d characters long. ", m_min)) % m_min
            << std::endl;
    else if (a_l_response.length() > m_max)
        std::cout
            << boost::format(gettext("Invalid input. "))
            << boost::format(ngettext("Enter a string at most %d character long. ", "Enter a string at most %d characters long. ", m_max)) % m_max
            << std::endl;
    else
        return true;
    return false;
}
// ----------------------------------------------------------------------------
template<>
bool Ask<int>::validate(std::string a_l_response)
// ----------------------------------------------------------------------------
{
    int intAnswer;
    if (!(std::stringstream(a_l_response) >> intAnswer))
        std::cout
            << gettext("Invalid input. ")
            << gettext("Enter an integer.")
            << std::endl;
    else if (intAnswer < m_min)
        std::cout
            << boost::format(gettext("Invalid input. "))
            << boost::format(gettext("Enter at least %d. ")) % m_min
            << std::endl;
    else if (intAnswer > m_max)
        std::cout
            << boost::format(gettext("Invalid input. "))
            << boost::format(gettext("Enter at most %d. ")) % m_max
            << std::endl;
    else
        return true;
    return false;
}
share|improve this question
    
Are you asking for something like this? –  jliv902 Feb 23 at 22:05
    
Show some more of AskUI please. I would like to see how it's templated, method declarations, etc. I'll say for now, store your AskBase*s in std::unique_ptr<AskBase>. Raw pointers should never own anything. –  David Feb 24 at 2:35

1 Answer 1

up vote 1 down vote

This is a good time to remind everybody of the importance of minimal examples.

#include <vector>

template<class Container> class AskUI {
    Container& m_ask;
public:
    AskUI(Container& a_ask) : m_ask(a_ask) {}
    int foo(int idx) { return m_ask.at(idx); }
};

int main() {
    std::vector<int> questions = { 1, 2, 3 };
    AskUI<std::vector<int>> ui(questions);
    ui.foo(2);
}

Your question is, can we somehow remove the requirement that questions be a named variable in this code? Well, the short answer is "no"; AskUI's constructor takes a Container by non-const lvalue reference, and so we have to have a non-const lvalue for it to refer to. What's more, in order to keep ui.m_ask from being a dangling reference, we need the referenced Container to live at least as long as ui itself. Otherwise, we could get away with something like

// Compiles, but yields undefined behavior (on OS X, throws std::out_of_range)
int main() {
    AskUI<const std::vector<int>> ui({ 1, 2, 3 });
    ui.foo(2);
}

The idiomatic C++11 way to write AskUI would be to have it make a copy of the passed-in Container, so that you could just pass in an anonymous (rvalue) Container and not have to worry about lifetimes. The AskUI itself would own and manage the lifetime of its Container.

template<class Container> class IdiomaticAskUI {
    Container m_ask;  // is now a value, not a reference!
public:
    template<class... Args>
    IdiomaticAskUI(Args&&... args) : m_ask{std::forward<Args>(args)...} {}
    int foo(int idx) { return m_ask.at(idx); }
};

int main() {
    IdiomaticAskUI<std::vector<int>> ui({ 1, 2, 3 });
    ui.foo(2);

    // And if you really NEED reference semantics, that's doable too:
    std::vector<int> questions = { 1, 2, 3 };
    IdiomaticAskUI<std::vector<int>&> ui2(questions);
    ui2.foo(2);
}
share|improve this answer
    
Please feel free to use minimal examples in your answer, but Code Review questions should not be reduced to minimal examples, else we wouldn't really be reviewing code anymore. –  200_success♦ Feb 25 at 9:40
    
Fair enough, I had forgotten that this was on CodeReview. The OP's question (As questions is not directly used in the routine later it seems like there should be a way to eliminate the variable by just passing the std::vector<AskBase*> directly [to] the AskUI constructor. Are there suggestions for removing it or rationale as to the necessity of it staying? I'm looking for C++14 and/or C++1z answers and any other comments.) would have fit in better on StackOverflow, and I answered it (semi-accidentally) with a StackOverflow answer. –  Quuxplusone Feb 25 at 18:42

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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