Best practice on long lists of constants

[ftl25]

Hi. I am a C/C++ newbie. I am looking for advice on best practice.
I am building an application which will take an input command (string), compare it with a list of stored commands, and go off to the relevant function. What i'd like to know is; what is the best way to declare/initialise the list of commands to compare against? Should I create a struct in the header and then initialise in a constructor or something like that?
I am using a switch statement, so they must be constant (ie I can't just use the relevant string within the switch statements), so I need to create a construct of some sort. There are commands in different categories. For example, IO commands, UUT commands etc. Our application can use C++ constructs but only C functions (for compatibility).

A code snippet showing what i'd like to do:

switch(sCmdInt[1]) 
	{
        // PEB 2 RELATED COMMANDS
	case  "EXECUTEFILESVF":
	TSL1_ExecuteWithVCCCheckCommand (sCmdInt[1])
        }

Any help would be appreciated.

[Ketsuekiame]

I would use a function lookup table.

It can be done with C/C++ compatibility.

Here's an example:

#include <iostream>
#include <string>

void FunctionOne()
{
   cout << "Function One Called" << endl;
}

void FunctionTwo()
{
   cout << "Function Two Called" << endl;
}

struct 
{
   (void)(*function)();
   const char* functionName;
}
lookUpTable[] = { {&FunctionOne, "FunctionOne"}, {&FunctionTwo, "FunctionTwo"}, {NULL, NULL} }; // Two null's to indicate end of array...

bool CallFunction(const string& functionName)
{
   for(int index = 0; lookUpTable[index].fn; index++)
   {
      if(functionName == lookUpTable[index].key)
      {
         (*(lookUpTable[index].fn))();
         return true;
      }
   }
   return false;
}

int main(int argc, char** argv)
{
   CallFunction("FunctionOne");
   CallFunction("FunctionTwo");
   return EXIT_SUCCESS;
}

There you have it. I know this is basically a code dump so I'll explain what it all does.

At the top you have two functions declares, as this is C-Style I've declared them first. All they do is print out a line of text.

The struct that comes underneath is where it starts getting interesting.
The struct declares a function pointer and a key.

This is a handy site to learn about function pointers

The CallFunction method simply looks for the key in the lookup table and calls the function pointer if it's found.

You can expand on this if you like by making the lookup table a vector or list, allowing you to dynamically add and remove function pointers as and when you require.

[mike_2000_17]

I am using a switch statement, so they must be constant

Then don't use a switch statement. The switch statement is just shorter (a bit) to write then "if () {} else if () {} ...", but it's fundamentally the same. So if it is not convenient for you, don't use it, it doesn't make the slightest difference at the end.

I don't recommend you use the code from Ketsuekiame (for many reasons I won't get into). But if you want to implement a similar strategy and I understand you are allowed to use some C++ constructs, then use "std::map" in the standard header "#include <map>". You can make it as this (in the flavor of Ketsuekiame):

#include <iostream>
#include <string>

bool FunctionOne() {
   cout << "Function One Called" << endl;
};

bool FunctionTwo() {
   cout << "Function Two Called" << endl;
};

typedef (bool)(*commandFnPtr)();

std::map< std::string, commandFnPtr > lookUpTable;

bool ExecCommand(const std::string& functionName) {
  std::map< std::string, commandFnPtr >::iterator iter = lookUpTable.find(functionName);
  if(iter != lookUpTable.end())
    return (*(iter->second))();
  return false;
};

int main(int argc, char** argv)
{
  lookUpTable["command_string1"] = &FunctionOne;
  lookUpTable["command_string2"] = &FunctionTwo;

  for(int i=1;i<argc;++i) {
    if(!ExecCommand(argv[i])) {
      std::cout << "An error occurred during command '" << argv[i] << "'. Terminating application..." << std::endl;
      return 1;
    };
  };
  return 0;
};

But, to be even more on the C++ side, you can use function objects (because function pointers are essentially deprecated in C++). And you will get nice additional benefits with it such as data members that can be used a parameters to the function object. In OOP spirit, I could recommend something more in the tune of this:

#include <iostream>
#include <string>
#include <map>

//base class for all your commands.
struct command {
  int errorCode; //report an error through a code, since you are leaning towards C, I didn't use exceptions.
  std::string errorMessage; //a bit more detail on errors is usually very nice to have.
  command() : errorCode(0), errorMessage("") { };
  virtual ~command() { };
  virtual void execute() = 0; //pure virtual function to execute the commands.
};

struct first_command : public command {
  //.. maybe some settings or parameters 
  first_command(.. maybe some initial settings here ..) { };
  static const std::string name = "first_command_name"; //store the command name as static const string member.
  virtual void execute() { //virtual method that executes the command.
    std::cout << "this command always fails!" << std::endl;
    errorCode = 1;
    errorMessage = "this was inevitable!!";
  };
};

struct second_command : public command {
  //.. maybe some settings or parameters 
  second_command(.. maybe some initial settings here ..) { };
  static const std::string name = "second_command_name"; //store the command name as static const string member.
  virtual void execute() {  //virtual method that executes the command.
    std::cout << "this command always works!" << std::endl;
    errorCode = 0;
  };
};

//have some global lookUpTable (this is not very nice but it'll work).
std::map< std::string, command* > lookUpTable;

int main(int argc, char** argv)
{
  //create the command list.
  lookUpTable[first_command::name] = new first_command(.. initial settings ..);
  lookUpTable[second_command::name] = new second_command(.. initial settings ..);

  //process the arguments
  int errorCode = 0;
  for(int i=1;i<argc;++i) {
    std::map< std::string, command* >::iterator iter = lookUpTable.find(argv[i]); //find the command object 
    if(iter != lookUpTable.end()) {
      iter->second->execute();   //execute the command.
      if(iter->second->errorCode) {
        //report an error with the command if there was one.
        std::cout << "Command failed with error " << iter->second->errorCode << " '" << iter->second->errorMessage << "'" << std::endl;
        errorCode = iter->second->errorCode;
        break; //stop the loop and go to clean up. (remove if you want to process more commands anyways)
      };
    } else {
      //What!?! that command does not exist!
      std::cout << "Error! Command not found!" << std::endl;
      errorCode = -1;
      break;
    };
  };

  //Always clean up..
  std::map< std::string, command* >::iterator iter = lookUpTable.begin();
  for(;iter != lookUpTable.end();++iter)
    delete iter->second;
  lookUpTable.clear();
  return errorCode;
};

Wow, maybe that was too much code dumping on my part.. hope this is self-explanatory enough.

[mrnutty]

>>it[switch statements] doesn't make the slightest difference at the end

On the contrary it does. Switch statements are easier to read, cleaner, and faster
than ifs/else's.

>>But, to be even more on the C++ side, you can use function objects

And to be even more more towards C++ side, you can ditch the new and delete command
for now, and let smart pointers handle it.

[mike_2000_17]

>> >>it[switch statements] doesn't make the slightest difference at the end

>>On the contrary it does. Switch statements are easier to read, cleaner, and faster
>>than ifs/else's.

OK well, to me that's a matter of opinion.. frankly I have never understood the purpose of switch statements in an object-oriented software design, and I (almost) never use them. So, I never liked them, never got used to them, and never found them easier to read or cleaner, I just find them useless and cumbersome. But you're right, they are faster, for contiguous integer case-values (or enums, of course), but not for strings (like in the OP) and most other types.

>> >>But, to be even more on the C++ side, you can use function objects

>>And to be even more more towards C++ side, you can ditch the new and delete command
>>for now, and let smart pointers handle it.

Sure that's a good point, and "delete" statements are a very rare occurrence in modern C++ code with smart pointers (in mine also, of course). But I omitted that point for a good reason: I like to preach awareness of memory management issues as a vital part of the understanding of computer programming, before giving more advanced tips. Crawl -> Walk -> Run.

[mrnutty]

>>OK well, to me that's a matter of opinion.. frankly I have never understood the purpose of switch statements in an object-oriented software design, and I (almost) never use them. So, I never liked them, never got used to them, and never found them easier to read or cleaner, I just find them useless and cumbersome. But you're right, they are faster, for contiguous integer case-values (or enums, of course), but not for strings (like in the OP) and most other types.<<

I guess I find this easier :

int state = event.state();
switch(state){
 PLAY_GAME : game.play(); break;
 PAUSE_GAME: even.pause(game); break;
 EXIT_GAME: event.cleanup(game); break;
}

that this :

int state = event.state();
if(state == PLAY_GAME){
  game.play();
}
else if(state == PAUSE_GAME){
  event.pause(game);
}
else if(state == EXIT_GAME){
 event.cleanup(game); break;
}

>>I like to preach awareness

Ok, I'm fine with that. But maybe mentioning that using new and delete is error prone,
especially to beginners, and thus suggesting to use smart pointers, would be a even
better post( IMO ), and not to mention a good advice.

>>Crawl -> Walk -> Run

Well, you should how to crawl, then walk, so why omit Run?

Anyways, I'm not trying to start a debate to take over OP's thread. So lets stop this.
If you want, you can send me a message.

[ftl25]

For one reason or another, I've been advised not to use lookup tables. For a start I can't use iostream (not linux compatible). Apologies, I didn't make it clear in the OP, I can only use C library functions - what I meant to say is that I can use the object orientation provided by C++ (classes etc). Also, with regard to the problem, each command function may take a different number of parameters depending on the command (I believe when using function pointers all functions must have the same parameters and return type). So the original problem remains.
Where is the best place to declare and initialize the commands, or any other suggestions would be gratefully received.

[mike_2000_17]

>>I can only use C library functions
Well in the posted examples, you can reasonably easily replace <string> with <string.h> and <iostream> with <stdio.h> and <stdlib.h>. And then the functionality is similar but you need to deal with char* instead of std::string and printf / scanf instead of std::cout and std::cin. That's no big deal, but a little more work of course, but you have no choice. The only problem is in my example where a std::map is used as a lookup-table. For that you can make your own lookup-table like in the first post from Ketsuekiame. But I don't understand why the <map> header cannot be included in your code because it's STL, which only needs you to have the headers (there are essentially no pre-compiled code attached to it) so if you can use C++ syntax, but no C++ standard library, well.. <map> is not, strictly-speaking, part of the "standard library", it's part of "standard TEMPLATE library" (a headers-only library that can compile on any naked environment with a C++ compiler and the STL headers).

>>I've been advised not to use lookup tables.
How else are you going to find the command that corresponds to a string? Unless you want to shackle yourself in the rigid format of a switch statement. The only better way than a naive lookup table is a hash table, and that is what std::map implements. Lookup tables are not recommended in the sense that they are usually not the nicest way to do anything, but a string is really not a nice way to identify a command (an int, an enum, or a pointer to a function object are usually more computer-friendly) but the problem is that if the command comes from a human, you kinda have to make it human-friendly, which usually leads to lookup tables and other less-than-desirable programming patterns.

>>different number of parameters depending on the command
Where do you get the parameters from? If you get it from the command line arguments, then I would assume you have something like "./exec_name firstcommand param1 param2 secondcommand param1 param2 param3 ..". In that case, after reading the command name, relay the pointer to the next argument to your command's function and process the parameters needed inside the command function, and keep track of the progression through the command-line arguments, this way all command function can have the same prototype.
If you prompt for the parameters, then prompt inside the command's function.
If you pass parameters internally from somewhere else in the code, then you can make those parameters data members of the function object and set them when you create the instance. If they need to change later, provide mutators (methods to set those parameters). Finally, Generic programming concepts can also help...

Function objects allow you to put parameters as data members that you set before or just bundle with the objects, and return types can also be data members that change state during the execute() method, and can be retrieved later.

You will need to give more specifics on where your parameters come from to get better suited help.. but there is always a way to do what you want in C++. (even in C libraries only).

[Ancient Dragon]

>>I can't use iostream (not linux compatible)
Who the hell told you that lie? (its just a rhetorical question, not meant to be answered here)

So you mean that you are really writing a C program, not c++. In that case std::map and std::string can not be used.

As for switch statements, I agree with firstPerson for simple cases. Switch statements only work with integers, not strings.

Using lookup table as described by Ketsuekiame would be the most efficient way to solve the problem, assuming all functions have the same parameters.

That leaves only one other alternative -- a series of if statements.

[mike_2000_17]

>> >>I can't use iostream (not linux compatible)
>>How the hell told you that lie?
It may not be a lie.. I suspect that the OP is running this code on a very peculiar platform (e.g. micro-controller) that could have a very non-standard implementation of Linux, because otherwise why would he not be able to use C++ libraries?

>>That leaves only one other alternative -- a series of if statements.
Lets not give up so fast.. object-oriented programming still has plenty of tricks that could help in this case, if we only knew enough about the case.

[ftl25]

mike, you are right - we test hardware, not just one peculiar type, but several - including CRAY (they don't even want to use a dev environment), linux, intel and any other processor you can think of. I have the job of taking a test script file and parsing it. The file could have any number of tests, each of which have any number of commands. The commands are in the format:
CommandName Param1 Param2 Param3

Different commands take different numbers of parameters.
I am at the point where I have each test in a structure and the command in an array. No command takes more than 6 parameters.
E.g.,
cmdArray[ReadMemory8, Address, Mask, Expected_Data]
I will read cmdArray[0] to determine which function to use.

[Ancient Dragon]

If he can only use C constructs then c++ object oriented programming is not useable ether. This thread should be in the C forum, not c++.

If the program is running on a non-standard linux, then my statement is still true. He's running non-standard linux, which is not the same as linux. I've had to work with versons of MS-Windows on embedded systems that didn't support C's FILE structure and related functions or c++ fstream -- had to use win32 api file functions. But that wasn't a true MS-Windows OS either.

If the OP is talking about embedded programming then he needs to say so. Embedded C and C++ compilers do not have to adhere to the same standards as other c/c++ compilers.

[Ketsuekiame]

In a case such as this one, although limited use of C++ may be allowed, unless you yourself can decide a clear cut case of when or when not to use C++ I would stick to writing it completely in C.

In which case I would use the Lookup Table template I gave you and alter it to fit your requirements. I specifically wrote it to be more C compatible than C++ oriented. There are a lot of C++ optimisations you can make to it, but I don't know if they're allowed.

In any case, that or continue with your switch statements, there aren't really any other options.

[ftl25]

@Ketsuekiame:
I've been following the tutorial you posted, and it seems to be more or less what I need, but I'm now having compilation errors.
A code snippet from the tutorial:

void Switch_With_Function_Pointer(float a, float b, float (*pt2Func)(float, float))
{
   float result = pt2Func(a, b);    // call using function pointer

   cout << "Switch replaced by function pointer: 2-5=";  // display result
   cout << result << endl;
}

My equivalent:

void TSL1_ExecuteTest (char *sCmdInt, char (*pt2Func)(char))
{
	// do something here later
}

Tutorial's call to function:

Switch_With_Function_Pointer(2, 5, /* pointer to function 'Minus' */ &Minus);

My call to function:

TSL1_ExecuteTest(sCmdInt, &sCmdInt[0]);

With the call above I am getting a compilation error as follows:
TSL1_ExecuteTest' : cannot convert parameter 2 from 'char *' to 'char (__cdecl *)(char)'

I can't see much different (apart from the use of chars instead of floats) in the two examples, so can anyone explain the compilation error?

[mrnutty]

>>cannot convert parameter 2 from 'char *' to 'char (__cdecl *)(char)'

You are trying to convert a char* to a function pointer.

[ftl25]

ok so what is the solution to this problem? As I understand it, the point of using a function pointer is that I can replace (*pt2Func) with a string pointer - the name of my function?

[mrnutty]

ok so what is the solution to this problem? As I understand it, the point of using a function pointer is that I can replace (*pt2Func) with a string pointer - the name of my function?

No. You need to pass it a function with the same prototype as your function pointer.

[Ketsuekiame]

firstPerson is correct, if you wanted to use Strings, you need to use the lookup table like I posted. What that does is give you two fields. One field is the name of the function (the string) and the other field is the function pointer itself.

Therefore, when you call "CallFunction" with the name of the method, it goes to the lookup table, finds the string and calls the function that is pointed to.

[mike_2000_17]

There is a another, a bit twisted way of achieving the lookup table without having to implement a lookup table: using a shared library (.so) and linking dynamically to it. This way you can use dlsym(handle, some_null_terminated_string_command_name) to get the function pointer for your command. Then all you need to do is put all your command functions inside a shared library and export them with their proper name.

To pass multiple parameters, take example of the main(int argc, char** argv) prototype. If you make all your command functions with the same format as that. You can parse the command-file just like the linux terminal parses them before launching the executable (space-separated arguments with special handling of double-quotes) and send the list of arguments to your commands and allow them to parse them internally for what ever data they need.