1
\$\begingroup\$

I have the following code that converts a couple of bash commands into Windows cmd commands, as well as adding some commands. This is only my second day programming in C++, so any and all suggestions would be helpful.

#include "stdafx.h"
#include <iostream>
#include <string>
#include <regex>
#include <fstream>
#include <stdlib.h>
#include <time.h>
#include <conio.h>

using namespace std;
using namespace std::tr1;

class Parser
{
    regex ls_regex, mkdir_regex, sleep_regex, rand_regex, rm_regex, touch_regex, cat_regex, open_regex, cp_regex, lat_regex;
    string touch_match, sleep_match, mkdir_match, ls_match, rm_match, open_match, cp_match;
   public:
    void SetRegexes();
    void RunCommand(string);
    string ParseCommand(string);
    string SearchString(int, string);
};

void Parser::SetRegexes()
{
    // Regex is the bash command, match is the cmd command
    ls_regex = "^ls$";
    ls_match = "dir";
    mkdir_regex = "^mkdir";
    mkdir_match = "md";
    sleep_regex = "^sleep$";
    sleep_match = "pause";
    rand_regex = "^timerand$";
    rm_regex = "^rm";
    rm_match = "del";
    touch_regex = "^touch";
    cat_regex = "^cat";
    open_regex = "^open";
    open_match = "notepad";
    cp_regex = "^cp";
    cp_match = "copy";
    lat_regex = "^lat";
}

string Parser::ParseCommand(string unparsed)
{
    // Checks if the regex can be found in the unparsed string
    if (regex_search(unparsed, ls_regex))
    {
        // Unparsed now is actually parsed
        unparsed = unparsed.replace(0, 3, ls_match);
    }
    else if (regex_search(unparsed, mkdir_regex))
    {
        unparsed = unparsed.replace(0, 5, mkdir_match);
    }
    else if (regex_search(unparsed, sleep_regex))
    {
        unparsed = unparsed.replace(0, 4, sleep_match);
    }
    else if (regex_search(unparsed, rand_regex))
    {
        srand(time(NULL));
        cout << rand() << endl;
        return " ";
    }
    else if (regex_search(unparsed, rm_regex))
    {
        unparsed = unparsed.replace(0, 2, rm_match);
    }
    else if (regex_search(unparsed, touch_regex))
    {
        string tfile;
        for (int i = 0; i < unparsed.length(); i++)
        {
            if (i > 5)
            {
                tfile += unparsed[i];
            }
        }
        ofstream create_file(tfile);
        return " ";
    }
    else if (regex_search(unparsed, cat_regex))
    {
        string op_line;
        string catfile;
        for (int i = 0; i < unparsed.length(); i++)
        {
            if (i > 3)
            {
                catfile += unparsed[i];
            }
        }
        ifstream input_file(catfile);
        while (input_file.good())
        {
            getline(input_file, op_line);
            cout << op_line;
        }
        cout << endl;
        return " ";
    }
    else if (regex_search(unparsed, open_regex))
    {
        unparsed = unparsed.replace(0, 5, open_match + " ");
    }
    else if (regex_search(unparsed, cp_regex))
    {
        unparsed = unparsed.replace(0, 4, cp_match + " ");
    }
    else if (regex_search(unparsed, lat_regex))
    {
        string latfile;
        string op_line;
        char holder;
        for (int i = 0; i < unparsed.length(); i++)
        {
            if (i > 3)
            {
                latfile += unparsed[i];
            }
        }
        cout << latfile << endl << endl;
        ifstream input_file(latfile);
        while (input_file.good())
        {
            getline(input_file, op_line);
            cout << op_line << endl;
            // getch() makes it so the letters are not echoed to the keyoard
            holder = getch();
        }
        return " ";
    }
    return unparsed;
}

void Parser::RunCommand(string parsed)
{
    // Converts to a different type so it can be used in the system method
    const char *cparsed = parsed.c_str();
    system(cparsed);
}

void _tmain(int argc, _TCHAR* argv[])
{
    string name;
    int line_counter = 0;
    Parser main_parser;
    main_parser.SetRegexes();
    string command;
    // Provides a never-ending command line interface
    while (true)
    {
        line_counter++;
        cout << "<" << line_counter << name << "-> "; 
        //cin >> command;
        getline(cin, command);
        command = main_parser.ParseCommand(command);
        main_parser.RunCommand(command);
    }
}
\$\endgroup\$
1
\$\begingroup\$

Some quick suggestions:

  • Since SetRegexes() does not change the values of your data members, better make the data members constant and initialize them in the constructor's initializer list. Thus you save some runtime in setting the values at initialization.

    private:
         const regex ls_regex ...
    public:
         Parser() : ls_regex("^ls$"),
            mkdir_regex("^mkdir"), ...
    
  • The function string Parser::ParseCommand(string unparsed) can be converted to void Parser::ParseCommand(string& unparsed) since what you are doing is modifying the input unparsed. You don't need to return the input when it is a reference.

  • I would suggest breaking the function ParseCommand(string unparsed) into two parts. First parts converts the input to a corresponsing enum. The second part switches this enum to the appropriate step. This way it is easier to read. Refer to this for the benefits of using switch over if-else: https://stackoverflow.com/questions/1028437/why-switch-case-and-not-if-else-if.

    typedef enum Ens {
       enLSRegex = 0,
       enLSMatch = 1, 
       ...
               enInvalid
    } Ens;
    
    Ens Parser::getEns( const string unparsed )
    {
        Ens en = enInvalid;
        if (regex_search(unparsed, ls_regex))
           en = enLSRegex ;
        else if(regex_search(unparsed, mkdir_regex))
           en = enLSMatch ;
        ...
        return en;
    }
    
    void Parser::ParseCommand(string& unparsed)
    {
        switch( getEns(unparsed) )
        {
            case enLSRegex:
               unparsed = unparsed.replace(0, 3, ls_match);
               break;
            case enLSMatch:
               unparsed = unparsed.replace(0, 5, mkdir_match);
               break;
               ...
        }
    }
    
  • Use updated versions of the libraries. Use cstdlib, ctime, conio for stdlib.h, time.h and conio.h respectively.

\$\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.