0

I am trying to write a command line interface for a school project. I have a driver.cpp to run the command line and that parses the commands into an array of strings for the commandInterpreter. When I try to debug it looks like only the first string is getting passed when I call interpreter->interpret(rowData, length); If I type "import p filename" in the command line, it looks like args only contains the string "import". I am expecting it to be passed {"import", "p", "filename"}. Am I passing or accessing the array of strings incorrectly?

Driver.cpp

#include "CommandInterpreter.h"
#include <iostream>
#include <sstream>
#include <string>

using namespace std;

int main() 
{
   CommandInterpreter * interpreter = new CommandInterpreter();

   while ( !interpreter->terminate() )
   {
      string cmd;
      getline(cin, cmd);

      //Parse command args to lowercase
      int i = 0;
      while(cmd[i])
      {
         char c = cmd[i];
         putchar(tolower(c));
         i++;
      }
      //Parse command line into command and parameters.
      //Space delimited.
      stringstream ss(cmd);
      string item;
      string * rowData = new string[100];
      int length = 0;
      while (getline(ss, item, ' ')) {
         rowData[length] = item;
         length++;
      }

      //Interpret command
      interpreter->interpret(rowData, length);
      //Print feedback
      cout << interpreter->feedback() << '\n';
   }

   delete interpreter;
   return 0;
}

CommandInterpreter.h

#ifndef __COMMANDINTERPRETER_H
#define __COMMANDINTERPRETER_H

#include <string>
#include "ProjectController.h"
#include "LocationController.h"
#include "VolunteerController.h"
#include "ScheduleGenerator.h"
#include "VolunteerScheduler.h"

class ProjectController;
class LocationController;
class VolunteerController;
class ScheduleGenerator;
class VolunteerScheduler;

using namespace std;

class CommandInterpreter
{
public:
   CommandInterpreter(void);
   ~CommandInterpreter(void);

   //---------------------------------------------------------------------
   // Tells the driver when to teminate the program.
   //---------------------------------------------------------------------
   bool terminate(void);

   //---------------------------------------------------------------------
   // Interprets the given array of commands.
   //---------------------------------------------------------------------
   bool interpret(string * commands, int length);

   //---------------------------------------------------------------------
   // Provides feedback for the last command run.
   //---------------------------------------------------------------------
   string feedback(void);

private:
   bool exitFlag;
   string handback;

   ProjectController * PC;
   LocationController * LC;
   VolunteerController * VC;
   ScheduleGenerator * SG;
   VolunteerScheduler * VS;

public:
   //---------------------------------------------------------------------
   // Interprets a save command.
   //---------------------------------------------------------------------
   bool save(char arg0);

   //---------------------------------------------------------------------
   // Interprets an import command
   //---------------------------------------------------------------------
   bool import(char arg0, string filename);

   //---------------------------------------------------------------------
   // Interprets a print command.
   //---------------------------------------------------------------------
   bool print(char arg0, char arg1);

   //---------------------------------------------------------------------
   // Interprets a schedule command.
   //---------------------------------------------------------------------
   bool schedule(char arg0);

   //---------------------------------------------------------------------
   // Interprets a help command. Provides a list of commands in feedback.
   //---------------------------------------------------------------------
   bool help(void);
};

#endif

CommandInterpreter.cpp

#include "CommandInterpreter.h"

CommandInterpreter::CommandInterpreter(void)
{
   exitFlag = false;
   handback = "";
   PC = new ProjectController();
   LC = new LocationController();
   VC = new VolunteerController();
   SG = new ScheduleGenerator();
   VS = new VolunteerScheduler();
}


CommandInterpreter::~CommandInterpreter(void)
{
   delete PC;
   delete LC;
   delete VC;
   delete SG;
   delete VS;
}


bool CommandInterpreter::terminate(void)
{
   return this->exitFlag;
}


bool CommandInterpreter::interpret(string * args, int length)
{
   //args should be an array of strings like {"import","p","filename"}
   //Debug only shows "import"

   string cmd = args[0];
   handback = "";

   if(cmd == "exit")
   {
      exitFlag = true;
   }
   else if(cmd == "save")
   {
      this->save(args[1][0]);
   }
   else if(cmd == "import")
   {
      this->import(args[1][0], args[2]);
   }
   else if(cmd == "print")
   {
      if(length == 2)
         this->print(args[1][0], '0');
      else
         this->print(args[1][0], args[2][0]);
   }
   else if(cmd == "schedule")
   {
      this->schedule(args[1][0]);
   }
   else if(cmd == "help")
   {
      this->help();
   }
   else
   {
      this->handback = "Invalid Command. Type help to see commands.";
      return false;
   }

   return true;
}


string CommandInterpreter::feedback(void)
{
   return this->handback;
}


bool CommandInterpreter::save(char arg0)
{
   bool success = true;

   switch (arg0)
   {
   case 'p':
      // PC->save();
      handback = "Saved project schedule.";
      break;
   case 'v':
      // VC->save();
      handback = "Saved volunteer schedule.";
      break;
   default:
      // Uh-oh
      handback = "Invalid argument for save: " + arg0;
      success = false;
      break;
   }
   return success;
}

bool CommandInterpreter::import(char arg0, string filename)
{
   bool success = true;
   switch (arg0)
   {
   case 'p':
      PC->importCSV(filename);
      handback = "Imported projects file: " + filename;
      break;
   case 'l':
      LC->importCSV(filename);
      handback = "Imported locations file: " + filename;
      break;
   case 'v':
      VC->importCSV(filename);
      handback = "Imported volunteers file: " + filename;
      break;
   default:
      success = false;
      handback = "Invalid argument for import command: " + arg0;
      break;
   }
   return success;
}


bool CommandInterpreter::print(char arg0, char arg1)
{
   bool success = true;
   switch (arg0)
   {
   case 'p':
      // PC->print()
      // or project schedule print
      // depending on arg1
      if(arg1 == '0')
         handback = PC->getProjectStrings();
      else
         ;   //Print project schedule here.
      break;
   case 'l':
      // LC->print()
      break;
   case 'v':
      // VC->print()
      // or volunteer schedule print
      // depending on arg1
      break;
   default:
      success = false;
      handback = "Invalid argument for print command: " + arg0;
      break;
   }
   return success;
}


bool CommandInterpreter::schedule(char arg0)
{
   bool success = true;
   switch (arg0)
   {
   case 'p':
      // SG->generate()
      handback = "Project schedule generated.";
      break;
   case 'v':
      // VS->generate()
      handback = "Volunteer schedule generated.";
      break;
   default:
      success = false;
      handback = "Invalid argument for schedule command: " + arg0;
      break;
   }
   return success;
}


bool CommandInterpreter::help(void)
{
   return false;
}
6
  • Why all the pointers and new? Did somebody tell you to do that? Commented Apr 23, 2014 at 21:38
  • Looks like pointless objectification to me... Commented Apr 23, 2014 at 21:44
  • Looks like somebody is using Java or C# techniques in C++. Commented Apr 23, 2014 at 21:51
  • args is a variable, of type std::string*. I doubt it contains any string. Commented Apr 23, 2014 at 21:52
  • You know there's no garbage collector in C++? Commented Apr 23, 2014 at 21:58

1 Answer 1

1

Wow, so many issues, so little time...

Unnecessary dynamic memory allocation
CommandInterpreter * interpreter = new CommandInterpreter(); You use std::string without dynamic allocation, so why this one here? Also, you should use a unique_ptr or boost::scoped_ptr that has pointer and memory management handling if you must dynamically allocate.

If the object is to big for a local variable, declare it as static.

Using '\0' for std::string termination
The '\0' is a C-style string terminator. When you use getline, it fills a std::string object with text. The std::string does not use a nul character for string termination. Change your while loop to use iterators or make it a for loop with length:

for (unsigned int i = 0; i < cmd.length(); ++i)
{
  cmd[i] = std::tolower(cmd[i]);
}

But why use a loop when you can use std::transform to convert all the characters. Search the web or StackOverflow for "c++ transform tolower".

Pass std::vector not arrays
As you have found out, passing arrays to functions is difficult, especially the syntax. Vectors are so much easier:

std::vector<std::string> commands;  // Look Ma, no *new* operator.
CommandInterpreter(commands);

Passing arrays
Remember that an array name can decay to the address of the first slot. Also, you need to pass the capacity along with the number of elements in the array:

void CommandInterpreter(string * array, unsigned int items_in_array, unsigned int array_capacity);

Or

void CommandInterpreter(std::vector<std::string>& array); // Much simpler using vector.

File naming conventions
You use a different extension for C++ headers. The ".h" suffix has historically been for C language header files. For example, the following is valid in a C language ".h" file:

typedef struct class
{
  unsigned int cout;
  double       private;
} class_t;

Your header file would do strange things when compiled for C language.

Some conventions use ".hh", or ".hpp" to denote a header file is for C++.

Improve readability with variable names
The two letter variables are not readable. Choose longer variable names. If you chose two letter abbreviations because its too long to type, take a keyboarding class.

Use initialization lists
You can use the new operator in an initializations list, which reminds me...

Check memory allocation for errors
Every dynamic memory allocation needs to be checked for success. This can be performed by checking the pointer for NULL or catching an exception. Also verify your implementation set up on whether or not new throws an exception. Some compilers allow you to turn off the exception throwing.

Not every function needs to be in an object
Some object oriented languages enforce a rule that functions must be in a class. The C++ language is different, you can have functions outside of classes called "free standing functions". Try it!

Enough for now ...

Sign up to request clarification or add additional context in comments.

Comments

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.