SIC C command checker programming fragment

Question

This program gets a string then checks if that's a command by comparing it to the 8 options available. It also checks if it has parameters. It these things with the help of the getting to know the length and getting checking for string equality.

        /*  BASIC HELPERS START  */
        /*  BASIC LENGTH  */
//a simple function to get total length of a string/command
int helperLength(char a[])
{
    for(int c = 0; c<1000; c++)
        if (a[c] == '\0')
            return c;
}
        /*  BASIC STRING EQUALITY  */
//most equality comparison between 2 strings
//programmingsimplified basically gave EQ for the most part
int CCHelperEQ(char a[], char b[])
{
    int c = 0;
    while (a[c] == b[c]) 
    {
        if (a[c] == '\0' || b[c] == '\0')
            break;
        c++;
    }

    if (a[c] == '\0' && b[c] == '\0')
        return 0;
    else
        return -1;
}
/*  BASIC HELPERS END  */
/*  WITH and WITHOUT PARAMETERS START  */
    /*  COMMANDS WITH PARAMETERS   */
//check if the parameters have errors
//might need to use three new functions for each of the three parameters checkers
int CCHelperWP(char commWP[],char WCommand[], int Pstart, int Cend)
{//command with parameter, which command, start of parameters, end of command
//return 1 if paramets work
    int parC = Pstart;//parameter counter
    char string1[] = "load ";      //1
    char string4[] = "dump ";     //4
    char string6[] = "assemble ";  //6
    if (CCHelperEQ(commWP, string1) == 0)
    {
        while (parC < Cend)
        {//checks if the parameter can work for the command
            if (commWP[parC] == '\0')
                return 0;
            parC++;
        }
        return 1;
    }
    else if (CCHelperEQ(commWP, string4) == 0)
    {
        int dumpC=0;
        while (parC < Cend)
        {//checks if the parameter can work for the command
//dump needs two hex values but at this point I do not need to check the parameters are hex only that they exist so this is enough
            if (commWP[parC] == '\0' && dumpC == 0)
                return 0;
            else if (commWP[parC] == ' ' && dumpC == 0)
                dumpC++;
            else if (commWP[parC] == ' ' && dumpC == 1)
                return 0;
            parC++;
        }
        return 1;
    }
    else if (CCHelperEQ(commWP, string6) == 0)
    {
        while (parC < Cend)
        {//checks if the parameter can work for the command
            if (commWP[parC] == '\0')
                return 0;
            parC++;
        }
        return 1;
    }
    return 0;
}
    /*  COMMANDS WITHOUT PARAMETERS   */
//check the command we are using is one without parameters
//needed otherwise error where functions that need parameters could give a bug with the set-up on compare_command
int CCHelperWO(char commNP[])
{//command no parameter
    char string1[] = "execute";
    char string2[] = "debug";
    char string3[] = "help";
    char string4[] = "directory";   
    char string5[] = "exit";
    if (CCHelperEQ(commNP, string1) == 0)
    {
        return 1;
    }
    else if (CCHelperEQ(commNP, string2) == 0)
    {
        return 1;
    }
    else if (CCHelperEQ(commNP, string3) == 0)
    {
        return 1;
    }
    else if (CCHelperEQ(commNP, string4) == 0)
    {
        return 1;
    }
    else if (CCHelperEQ(commNP, string5) == 0)
    {
        return 1;
    }
    else return 0;
}
/*  WITH and WITHOUT PARAMETERS END  */
//compare_command main helper function
int CCHelper(char a[], char b[])
{
    int c = 0;
    int length = helperLength(a);
    while (a[c] == b[c]) 
    {
        if (a[c] == '\0' && b[c] == '\0')
        {//checks if the command is one that doesn't need parameters
            if(CCHelperWO(b) == 1)
            {    return 0;  }
            else return -1;
        }
        if (b[c] == '\0')
            if (1 == CCHelperWP(a, b, c, length))
                return 0;
        c++;
    }
}

int compare_command(char co[]) {
/* 
going to give an int value to each command
with lots of if statements to check if the words is the command
after the words is the command I will have other functions
that check for the different parameters required for each command.
*/
    char stringLF[] = "load ";      //1
    char stringExe[] = "execute";   //2
    char stringDeb[] = "debug";     //3
    char stringDSE[] = "dump ";     //4
    char stringH[] = "help";        //5
    char stringAF[] = "assemble ";  //6
    char stringDir[] = "directory"; //7
    char stringExit[] = "exit";     //8
    if (CCHelper(co, stringLF) == 0)
    {//With Parameter
        return 1;
    }
    else if (CCHelper(co, stringExe) == 0)
    {//Without Parameter
        return 2;
    }
    else if (CCHelper(co, stringDeb) == 0)
    {//Without Parameter
        return 3;
    }
    else if (CCHelper(co, stringDSE) == 0)
    {//With Parameter
        return 4;
    }
    else if (CCHelper(co, stringH) == 0)
    {//Without Parameter
        return 5;
    }
    else if (CCHelper(co, stringAF) == 0)
    {//With Parameter
        return 6;
    }
    else if (CCHelper(co, stringDir) == 0)
    {//Without Parameter
        return 7;
    }
    else if (CCHelper(co, stringExit) == 0)
    {//Without Parameter
        return 8;
    }
    else return 9;
}

I do not know if this programming fragment is a bit overcomplicated, or if using pointers would help. Any tips would be greatly appreciated since I feel I'm using too many functions for what I'm doing.

Answer 1

You say:

I feel I'm using too many functions for what I'm doing.

I think you're right. You appear to have reimplemented 2 standard library functions with minor changes. Your helperLength() function could be written as:

int helperLength(char a[])
{
    int result = strlen(a);
    if (result >= 1000)
    {
        result = 1000;
    }
    return result;
}

The question is, why are you limiting the length to be 1000 in this way? It seems like you could replace calls to helperLength() with a call to strlen() and do the limiting at the call site (if it even needs to be done) unless you're going to be reusing the function in lots of places.

Likewise, your CCHelperEQ function could be written as:

int CCHelperEQ(char a[], char b[])
{
    return strcmp(a, b) != 0;
}

Additionally, you seem to be doing the same work repeatedly. In CCHelperWP() the while loop in the first if block looks identical to the one in the last if block`. You should make it a function and call it from both. And it could also probably be replaced with this logic:

return strlen(&commWP[Pstart]) >= (Pend - Pstart);

You CCHelperWO() function makes good use of the CCHelperEQ() function. Nice reusability there! (But I'd still replace those calls with just a call to strlen().)

The CCHelper() function would be easier to read if you wrote it like this:

int CCHelper(char a[], char b[])
{
    if (strcmp(a, b) == 0)
    {
        if (CCHelperWO(b) == 1)
        {
            return 0;
        }
        return -1;
    }

    int length = strlen(a);
    int bLen = strlen(b);
    if (CCHelperWP(a, b, bLen, length) == 1)
    {
        return 0;
    }
    // DO SOMETHING HERE!
}

Notice that at the end, it's possible that you've reached a state where you should return something, but there's no return statement. That's an error and you need to put something there.

Finally your compare_command() function could be rewritten using a table driven design. It would look something like this:

int compare_command(char co[]) {
    const char* commands[] = {
        "load ",      //1
        "execute",   //2
        "debug",     //3
        "dump ",     //4
        "help",        //5
        "assemble ",  //6
        "directory", //7
        "exit"     //8
    };
    const numEntries = sizeof(commands) / sizeof(commands[0]);
    for (int i = 0; i < numEntries; ++i)
    {
        if (strcmp(co, commands [ i ]) == 0)
        {
            return i + 1;
        }
    }
    return numEntries + 1;
}

Once you've done all of that, I recommend rethinking your naming strategy. The names in your code are very difficult to read. Using the CC prefix is fine, since this is a "command checker". But names like CCHelperWO and commNP are very terse. I recommend using longer, more descriptive names, like CCParseCommandNoParam() and command for those particular names.

Also, you might want to rethink whether you should really have to handle commands with parameters differently from commands without parameters. It seems like a loop that gets the command and then checks how many parameters it should have, and then parses that many parameters would collapse this down to 1 code path for all commands.