Handling argument flags

There are plenty of ifs and elses, and someone else told me that using switch/case statements could really clean up the code. However, he didn't implement it for me, so I don't know what I should switch/case.

Also take note that the if (!stricmp(argv[arg], "-vmf")) and if (!stricmp(argv[arg], "-bsp")) blocks are exactly the same, minus the usage of vmf and bsp, and that the wad block is somewhat similar.

```
#include
#include
#include
#include

#include "arg.h"
#include "files.h"
#include "stricmp.h"

using namespace std;

//"ifError" takes all the argument flags, as well as a string to output. If silent flag is
//enabled, then "ifError" will not output that text. I put this into the function to reduce
//lines and characters of code.
void ifError(argFlags& argFlags, char* errorMessage)
{
if (!argFlags.sil)
{
if (!argFlags.err)
{
printf("Converts DOOM WAD into VMF and/or BSP file(s).\n\n");
printf("DOOM2BSP [-silent] [-WAD [path][name]] [-VMF [path]] [-BSP [path]]\n\n");
printf(" -WAD Location of WAD.\n");
printf(" -VMF Create VMF file(s) in path.\n");
printf(" -BSP Create BSP file(s) in path.\n");
printf(" -silent Disable text output.\n\n");
}
printf(errorMessage);
}
argFlags.err = true;
}

//Checks if directory exists.
bool checkDir(char* path)
{
struct stat st;
if (!stat(path, &st))
if (st.st_mode & S_IFDIR != 0)
return true;
return false;
}

//This function analyzes our arguments, deals with them accordingly, and decides if there
//are any errors or not. Also opens wad if it exists.
void handleArgs(argFlags& argFlags, files& files, int& argc, char**& argv)
{
//Analyzes all arguments to find "-silent" arg. If silent arg is found, the silent flag
//is turned on, signifying that this program should not output any text.
for (int arg = 1; arg < argc; ++arg)
i

I think you should separate the logic parsing the arguments provided from the logic using the argument parsed. At the moment, you are trying to do everything in one place and I'm not quite sure this is correct (for instance, if a filename is also an option name, this leads to an unexpected behavior).

I'd suggest something like (I'm not even sure it would compile but that just to give you an idea) :

void handleArgs(argFlags& argFlags, files& files, int& argc, char**& argv)
{
    int waitingForArg = 0; // To know if we are expecting an other argument to be provided : 0:no, 1:wad, 2:vmf, 3:bsp
    for (int i = 1; i < argc; i++)
    {
        char* arg = argv[i];
        switch (waitingForArg)
        {
            case 0:
                if (!stricmp(arg, "-silent"))
                    argFlags.sil = true;
                else if (!stricmp(arg, "-wad")
                    waitingForArg = 1;
                else if (!stricmp(arg, "-vmf")
                    waitingForArg = 2;
                else if (!stricmp(arg, "-bsp")
                    waitingForArg = 3;
                break;
            case 1:
                argFlags.wadFile = arg; waitingForArg = 0;
                break;
            case 2:
                argFlags.vmfFile = arg; waitingForArg = 0;
                break;
            case 3:
                argFlags.bspFile = arg; waitingForArg = 0;
                break;
        }
    }

    if (waitingForArg)
    {
        ifError(argFlags, "Error: file name must be provided with option -wad, -vmf or -bsp");
        return;
    }

    // Logic involving wadFile, wmfFile, bspFile. Checking if they are set and/or if they correspond to existing files.
}

This is just a first step to make things easier to follow (on top of being more correct and more efficient).

Then to remove code duplication, you might want to do something like using an array to store the different possible options (["-wad","-vmf","-bsp"]). In order to do so, you'd replace the "else if (!stricmp(arg, "OPTIONS"))" with a loops on the different options. Then you might also be happy to store the flags as an array. I think this would make more sense if you had more flags to handle.

I also notice that you have tagged your question as C++ but your code seems to be using some C-style (printf and stuff). This might be something you should fix as well.

StackExchange Code Review Q#21562, answer score: 8