3
\$\begingroup\$

I tried to parse command line arguments. The program requires four arguments. I iterate over the arguments. If the argument is an option, I process the option. Otherwise the argument is one of the required arguments. In order to read the required arguments I need some kind of state machine. In the first case the first argument should be read. In the second case the second argument and so on.

I wrote a Proc class with just one method, which returns again a Proc class.

static abstract class Proc {
  abstract Proc exec (String arg);
}

By this I can execute some action and define what should be done next.

  1. save db host and then read name
  2. save db name and then read user
  3. save db user and then read xml file
  4. save xml file and then nothing

But because of all the class overhead it is hard to read.

Proc proc = new Proc () {
    Proc exec (String arg) {
      db_host = arg;
      return new Proc () {
        Proc exec (String arg) {
          db_name = arg;
          return new Proc () {
            Proc exec (String arg) {
              db_user = arg;
              return new Proc () {
                Proc exec (String arg) {
                  xml_file = arg;
                  return null;
                }
              };
            }
          };
        }
      };
    }
  };

Is there a way to simplify the code? I tried Lambdas, but it seems to me that Lambdas can use only final variables, which is a bit useless, when I want to store a value.

Complete example:

public class Import
{
  static String db_host = null;
  static String db_port = "5432";
  static String db_name = null;
  static String db_user = null;
  static String xml_file = null;

  static void usage ()
  {
    System.err.println ("Usage: Import [-p PORT] HOST DATABASE USER FILE");
  }

  static abstract class Proc {
    abstract Proc exec (String arg);
  }

  static void parse_args (String[] args)
  {
    Proc proc = new Proc () {
        Proc exec (String arg) {
          db_host = arg;
          return new Proc () {
            Proc exec (String arg) {
              db_name = arg;
              return new Proc () {
                Proc exec (String arg) {
                  db_user = arg;
                  return new Proc () {
                    Proc exec (String arg) {
                      xml_file = arg;
                      return null;
                    }
                  };
                }
              };
            }
          };
        }
      };

    try {
      for (int i = 0; i < args.length; i++)
        switch (args[i]) {
        case "-p":
          db_port = args[++i];
          break;
        case "-h":
          usage ();
          break;
        default:
          proc = proc.exec (args[i]);
        }
    }
    catch (Exception ex) {
      throw new Error ("Can not parse args!", ex);
    }
  }

  public static void main (String[] args)
  {
    parse_args (args);

    System.err.println ("db_host: " + db_host);
    System.err.println ("db_port: " + db_port);
    System.err.println ("db_name: " + db_name);
    System.err.println ("db_user: " + db_user);
    System.err.println ("xml_file: " + xml_file);
  }
}
\$\endgroup\$
5
\$\begingroup\$

First, you should follow the Java naming conventions.


Second, don't shorten names just because you can. Example, what does "Proc" stand for? Procedure? Processor? Even if names become longer, the readability is worth it!


static String db_host = null;

Your members should not be static, and should ideally be qualified with something else than package-private, like protected or private to show the intent clearly.


In order to read the required arguments I need some kind of state machine.

Is that a requirement you've been given or is that an assumption you had? Because that's not true at all. Overall, your system is completely unnecessarily complex without an obvious reason or benefit. Neither does it seem like a state-machine at all, it's just an overly complex chain of function calls.

A state-machine for your command-argument parsing would be something like this:

for (int index = 0; index < args.length; index++) {
    String arg = args[index];
    
    if (arg.equals("-p") && databaseHost == null) {
        arg = args[++index];
        
        databasePort = arg;
    } else if (databaseHost == null) {
        databaseHost = arg;
    } else if (databaseUsername == null) {
        databaseUsername = arg;
    } else if (databasePassword == null) {
        databasePassword = arg;
    } else if (inputFile == null) {
        inputFile = arg;
    }
}

And even that is oddly complex.

What you want is the most simplest solution that works, and that would be a hardcoded extracting of arguments:

if (args[0].equals("-h")) {
    printHelp();
    return;
}

int portProvidedOffset = 0;

if (args[0].equals("-p")) {
    databasePort = args[1];
    
    portProvidedOffset = 2;
}

databaseHost = args[portProvidedOffset + 0];
databaseUsername = args[portProvidedOffset + 1];
databasePassword = args[portProvidedOffset + 2];
inputFile = args[portProvidedOffset + 3];

Which is, of course, not pretty, but the most simplest solution you can get away with. Of course, the length of the array should be checked beforehand, whether it is 4 elements or more. Afterwards you'd check whether all parameters are set, if not, exit with an error.

If you want a more sophisticated solution, you'd need to write yourself a complete argument parser, a not-so-trivial exercise.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ About "Proc". Everything is an abbreviation. "Proc" stands for: TheClassWithTheMethodExecTakingAnStringAndReturningAnInstanceOfTheSameClass. It is not a matter whether you abbreviate. It is just a matter how much you abbreviate. \$\endgroup\$ – ceving Aug 24 at 6:57
  • \$\begingroup\$ You can argue about it however you wish, my experience is that the more descriptive the names, the less abbreviations or "shortcuts" are used, the better is the readability and maintainability of the code. \$\endgroup\$ – Bobby Aug 26 at 16:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.