3
\$\begingroup\$

I am new to c++ and as a project to learn the language I have been working on a console application that would be able to handle and execute commands from the user. I have gotten it to work the way I want it to however I feel that I am doing it a very inefficient way (both handling the command and parsing the input into tokens). If someone could help point me in the right direction or give me some pointers this would be greatly appreciated.

console.h

    class console{
    public:
        console();
        ~console();

        //this will update the console application
        void update();

        //these wiill be used to print to the screen
        static void printl(const char* output);
        static void printl(std::string output);
        static void printl(int value);
        static void printl(float value);
        static void printl(double value);

        static void print(const char* output);
        static void print(std::string output);
        static void print(int value);
        static void print(float value);
        static void print(double value);

        //this will be used to take input from the screen
        void getInput();

        //clears the value of command
        void resetCommand();

        //breaks the command into its tokens
        void parseCommand(std::string command);

        //evaluates what it will do based on the command
        int evaluate();

        void exicuteCommand(int commandNum);

        //this will return the first token
        std::string getFirstToken();

        //sets active to false
        void exitConsole();

        //if the console is running or not
        bool isActive() const;
    private:
        bool active;
        std::string command;
        std::string tokens[256];
    };

console.cpp

#define OPTIONSSIZE 1024
std::string options[OPTIONSSIZE] = {
        "exit",     //0 exit console
        "cal",      //1 perform a calculation
        "clr",      //2 clear the screen
        "connect",  //3 connect to server
        "open",     //4 open a file
        "close"     //5 close a file or program
    };

    console::console(){
        active = true;
    }

    //------------------------------------------------------
    //This will be a loop that out and in puts to the screen
    //------------------------------------------------------
    void console::update(){         
        while (command == ""){
            getInput();
        }
        parseCommand(command);
        exicuteCommand(evaluate());
        resetCommand();
    }


    void console::getInput(){
        std::getline(std::cin, command);
    }

    void console::parseCommand(std::string command){
        char *myString = &command[0];
        char *p = strtok(myString, " ");
        int i = 0;
        std::stringstream ss;
        while (p) {
            //gets the current token and adds it to the string stream
            ss << p;
            //the string stream then converts it into a std::string and adds it to the token array
            ss >> tokens[i];
            //it then clears the stream, resetting it
            ss.clear();
            //removes the token from p
            p = strtok(NULL, " ");
            i++;
        }
    }

    std::string console::getFirstToken(){
        return tokens[0];
    }

    //------------------------------------------------------
    //evaluates what to do based on the tokes passed
    //------------------------------------------------------
    int console::evaluate(){
        //default
        int optionNum = -1; 
        //lookes through all options and compairs them to the focus command
        for (int i = 0; i < OPTIONSSIZE; i++){
            if (options[i] == getFirstToken()){
                optionNum = i;
                break;
            }
        }
        return optionNum;
    }

    void console::exicuteCommand(int commandNum){
        switch (commandNum){
        case 0:
            exitConsole();
            break;
        case 1:
        {
            Calculator cal(tokens);
        }
            break;
        case 2:
            system("cls");
            break;
        case 3:
            printl("Connecting with server");
            break;
        case 4:
            break;
        case 5:
            break;
        default:
            printl("Command not valid");
            break;
        }
    }


    void console::resetCommand(){
        command = "";
    }

    void console::exitConsole(){
        this->active = false;
    }

    bool console::isActive() const{
        return this->active;
    }

    void console::printl(const char* output){
        std::cout << output << std::endl;
    }

    void console::printl(std::string output){
        std::cout << output << std::endl;
    }

    void console::printl(int value){
        std::cout << value << std::endl;
    }

    void console::printl(float value){
        std::cout << value << std::endl;
    }

    void console::printl(double value){
        std::cout << value << std::endl;
    }

    void console::print(const char* output){
        std::cout << output;
    }

    void console::print(std::string output){
        std::cout << output;
    }

    void console::print(int value){
        std::cout << value;
    }

    void console::print(float value){
        std::cout << value;
    }

    void console::print(double value){
        std::cout << value;
    }

    console::~console(){
    }

int main()

int main(){
    console console;
    console.printl("Solution [Command Line]");
    console.printl("------Version 0.1-------");
    console.printl("");

    while (console.isActive()){
        console.print("Solution::$ ");
        console.update();
    }
    return 0;
}
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

Static members?

static void printl(double value);

Currently they all print to std::cout.
But I can see the processes of haveing multuple of these running with different streams connected to different standard points. Looks like these should be normal members rather than static members.

Hard code std::cout

void console::print(float value){
    std::cout << value;
}

Sure version 1 you may want these hard coded. But I would set this so you have a custom in/out streams that are set up in the constructor. By default these are std::cin/std::cout but I would not hard code that.

Templatize commong code

void console::print(float value){
    std::cout << value;
}
... Repeat 10 times.

You could just write a template:

template<typename T>
void console::print(T const& value){
    std::cout << value;
}

Public Interface

This seems like a very large surface area for a public interface:

    //this will update the console application
    void update();

    // Print
    ....

    //this will be used to take input from the screen
    void getInput();

    //clears the value of command
    void resetCommand();

    //breaks the command into its tokens
    void parseCommand(std::string command);

    //evaluates what it will do based on the command
    int evaluate();

    void exicuteCommand(int commandNum);

    //this will return the first token
    std::string getFirstToken();

    //sets active to false
    void exitConsole();

    //if the console is running or not
    bool isActive() const;

Are all those supposed to be public?
The main routine only uses two methods:

    // if the console is running or not
    bool isActive() const;

    //this will update the console application
    void update();

    // And print (but you could combine that into update).

Self Sizing arrays:

#define OPTIONSSIZE 1024
std::string options[OPTIONSSIZE] = {
        "exit",     //0 exit console
        "cal",      //1 perform a calculation
        "clr",      //2 clear the screen
        "connect",  //3 connect to server
        "open",     //4 open a file
        "close"     //5 close a file or program
    };

Or you could get a self sized array:

std::string options[] = {  // by not setting a size the compiler works it out from the parameters.
        "exit",     //0 exit console
        "cal",      //1 perform a calculation
        "clr",      //2 clear the screen
        "connect",  //3 connect to server
        "open",     //4 open a file
        "close"     //5 close a file or program
    };
// Calculate the size based on the actual size.
std::size_t constexpr OPTIONSSIZE = sizeof(options)/sizeof(options[0]);

Even better would be to use a standard container:

 std::vector<std::string> options = {
        "exit",     //0 exit console
        "cal",      //1 perform a calculation
        "clr",      //2 clear the screen
        "connect",  //3 connect to server
        "open",     //4 open a file
        "close"     //5 close a file or program
    };

Tokenzing a string

    char *myString = &command[0];
    char *p = strtok(myString, " ");

strtok() is not a very nice tokenizer (as it destroyes the input string). Much nicer (and easier) to use a stringstream and use operator>> to tokenize words.

    std::stringstream commandStream(command)

    std::string c1;
    commandStream >> c1;

Or if you just want to tokenize the whole thing in one go:

    std::stringstream commandStream(command);
    std::vector<std::string>   tokens(std::istream_iterator<std::string>(commandStream),
                                      std::istream_iterator<std::string>());

Use Sorted containers:

You are searching for a string manually (and serially).

    for (int i = 0; i < OPTIONSSIZE; i++){
        if (options[i] == getFirstToken()){
            optionNum = i;
            break;
        }
    }

If you use a sorted container for commands it will do the search for you.

    std::set<std::string>  options  = /* Initialized somehow */;

    ....

    std::set<std::string>::const_iterator find = options.find(getFirstToken());
    if (find != options.end())
    {
         //  Execute command
    }

Command Pattern

You can extend this to make the options hold the function to be exectuted.

    std::map<std::string, std::function<void(std::vector<std::string>)>>  options = { /* Initialize with the option and the function to execute */ };

    ....
    // Now you can search for the command.
    // If it is there just execute it directly
    auto const find = options.find(getFirstToken());
    if (find != options.end())
    {
         find->second(tokens); // execute the function.
    }
\$\endgroup\$

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.