Return to Answer

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: http://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.

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.

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: http://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;
              ...
       }
   }
  

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: http://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.

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 after 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: http://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;
              ...
       }
   }
  

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: http://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;
              ...
       }
   }