1

I'm having trouble creating clear and concise code that allows me to make a variety of commands to do a variety of different things. So for example, in an N-body simulator I am working on, the functionality that I want is that the user can enter a command like tele pos [x] [y] [z] or tele celobj [celestial object name].

To do this, I divide the input string into an array of tokens based on where the spaces are. Then, I use a series of switch statements such that the first word (tele) is processed in one layer of switch statements and then the second word (pos or celobj) is processed in a second layer of switch statements. Then the next tokens are processed accordingly. Through all of these different layers, I check to see that the user has entered a valid number of words to avoid an out of range exception.

My code works fine, but obviously it is very hard to read and overly complex. I'm not so much looking for code to help me, but a conceptual strategy for organizing the a command system or for setting up the logic in an optimal manner.

I have included my source code just in case, but I hope my description was clear enough.

public static void process(String cmd) {
    String tokenNotFound = "Token not recognized...";
    String notEnoughInfo = "Not enough info given. Please specify...";
    String unableToParse = "Unable to parse number...";

    String[] tokens = cmd.toLowerCase().split("\\s+");
    switch (tokens[0]) {
        case "close":
            run = false;
            break;
        case "toggle":
            if (tokens.length >= 2) {
                switch (tokens[1]) {
                    case "render":
                        render = !render;
                        System.out.println("Render setting set to " + render);
                        break;
                    case "physics":
                        updatePhysics = !updatePhysics;
                        System.out.println("Physics update setting set to " + updatePhysics);
                        break;
                    case "trails":
                        showTrails = !showTrails;
                        System.out.println("Show trails setting set to " + showTrails);
                        break;
                    case "constellations":
                        showConstellations = !showConstellations;
                        System.out.println("Show constellations setting set to " + showConstellations);
                        break;
                    default:
                        System.err.println(tokenNotFound);
                }
            } else
                System.err.println(notEnoughInfo);
            break;
        case "get":
            if (tokens.length >= 2) {
                switch (tokens[1]) {
                    case "fps":
                        System.out.println("FPS: " + realFPS);
                        break;
                    case "ups":
                        System.out.println("UPS: " + realUPS);
                        break;
                    case "cps":
                        System.out.println("CPS: " + realCPS);
                        break;
                    case "performance":
                        System.out.println("FPS: " + realFPS + " UPS: " + realUPS + " CPS: " + realCPS);
                        break;
                    case "time":
                        System.out.println(getTimestamp());
                        break;
                    case "celobj":
                        if (tokens.length >= 3) {
                            boolean objFound = false;
                            CelObj chosenObj = null;
                            for (CelObj celObj : physics.getCelObjs()) {
                                if (celObj.getName().toLowerCase().equals(tokens[2])) {
                                    objFound = true;
                                    chosenObj = celObj;
                                }
                            }

                            if (objFound) {
                                if (tokens.length >= 4) {
                                    switch (tokens[3]) {
                                        case "pos":
                                            Vec3d pos = chosenObj.getCelPos();
                                            System.out.println("POSITION: X= " + pos.x + " Y= " + pos.y + " Z= " + pos.z);
                                            break;
                                        case "vel":
                                            Vec3d vel = chosenObj.getCelVel();
                                            if (tokens.length >= 5 && tokens[4].equals("mag"))
                                                System.out.println("VELOCITY: V= " + vel.magnitude());
                                            else
                                                System.out.println("VELOCITY: X= " + vel.x + " Y= " + vel.y + " Z= " + vel.z);
                                            break;
                                        case "mass":
                                            System.out.println("MASS: M= " + chosenObj.getMass());
                                            break;
                                        case "radius":
                                            System.out.println("RADIUS: R= " + chosenObj.getRadius());
                                            break;
                                        default:
                                            System.err.println(notEnoughInfo);
                                    }
                                } else
                                    System.err.println(notEnoughInfo);
                            } else
                                System.err.println(tokenNotFound);
                        } else {
                            //Print list of celObjs
                            StringBuilder celObjNames = new StringBuilder("Celestial Objects: \n");
                            for (CelObj celObj : physics.getCelObjs()) {
                                celObjNames.append('\t').append(celObj.getName()).append('\n');
                            }
                            System.out.println(celObjNames.toString());
                        }
                        break;
                    default:
                        System.err.println(tokenNotFound);
                }
            } else
                System.err.println(notEnoughInfo);
            break;
        case "set":
            if (tokens.length >= 2) {
                switch (tokens[1]) {
                    case "cps":
                        if (tokens.length >= 3) {
                            try {
                                int newCPS = parseInt(tokens[2]);
                                realTime_to_simTime = newCPS * timeInc;
                                System.out.println("Target CPS set to " + newCPS);
                                System.out.println("The simulation time is " + realTime_to_simTime + " times the speed of real time");
                            } catch (Exception e) {
                                System.err.println(unableToParse);
                            }
                        } else
                            System.err.println(notEnoughInfo);
                        break;
                    case "scale":
                        if (tokens.length >= 3) {
                            try {
                                scale = parseFloat(tokens[2]);
                                System.out.println("Render object scale is now set to " + scale);
                            } catch (Exception e) {
                                System.err.println(unableToParse);
                            }
                        } else
                            System.err.println(notEnoughInfo);
                        break;
                    case "speed":
                        if (tokens.length >= 3) {
                            try {
                                speed = parseFloat(tokens[2]);
                                System.out.println("Speed is now set to " + speed);
                            } catch (Exception e) {
                                System.err.println(unableToParse);
                            }
                        } else
                            System.err.println(notEnoughInfo);
                        break;
                    case "record":
                        if (tokens.length >= 4) {
                            if (tokens[3].equals("period")) {
                                try {
                                    int newCPS = parseInt(tokens[2]);
                                    realTime_to_simTime = newCPS * timeInc;
                                    System.out.println("Target CPS set to " + newCPS);
                                    System.out.println("The recording period is now every " + realTime_to_simTime + " seconds");
                                } catch (Exception e) {
                                    System.err.println(unableToParse);
                                }
                            } else
                                System.err.println(tokenNotFound);

                        } else
                            System.err.println(notEnoughInfo);
                        break;
                    case "center":
                        if (tokens.length >= 3) {
                            boolean objFound = false;
                            CelObj chosenObj = null;
                            for (CelObj celObj : physics.getCelObjs()) {
                                if (celObj.getName().toLowerCase().equals(tokens[2])) {
                                    objFound = true;
                                    chosenObj = celObj;
                                }
                            }

                            if (objFound) {
                                centerCelObj = chosenObj;
                                System.out.println(chosenObj.getName() + " has been set as the center");
                            } else
                                System.err.println(tokenNotFound);
                        } else
                            System.err.println(notEnoughInfo);
                        break;
                    default:
                        System.err.println(tokenNotFound);
                }
            } else
                System.err.println(notEnoughInfo);
            break;
        case "create":
            //TODO:
            break;
        case "uncenter":
            centerCelObj = null;
            System.out.println("There is currently no center object");
            break;
        case "tele":
            if (tokens.length >= 2) {
                switch (tokens[1]) {
                    case "pos":
                        if (tokens.length >= 5) {
                            try {
                                double x = parseDouble(tokens[2]);
                                double y = parseDouble(tokens[3]);
                                double z = parseDouble(tokens[4]);

                                Vec3f cameraPos = new Vec3f((float) x, (float) y, (float) z);

                                //If camera is locked to an object, then translating the camera will only
                                //do so with respect to that planet
                                //Hence, the camera is translated back to world coordinates by translating it
                                //the negative of its locked celObj position vector
                                if (camera.getLockedCelObj() != null) {
                                    cameraPos.translate(
                                            new Vec3f(
                                                    camera.getLockedCelObj().getCelPos()
                                            ).negate()
                                    );
                                }

                                camera.setPosition(multiply(worldunit_per_meters, cameraPos));
                                System.out.println("The camera position has been set to X= " + x + " Y= " + y + " Z= " + z);
                            } catch (Exception e) {
                                System.err.println(unableToParse);
                            }
                        } else
                            System.err.println(notEnoughInfo);
                        break;
                    case "celobj":
                        if (tokens.length >= 3) {
                            boolean objFound = false;
                            CelObj chosenObj = null;
                            for (CelObj celObj : physics.getCelObjs()) {
                                if (celObj.getName().toLowerCase().equals(tokens[2])) {
                                    objFound = true;
                                    chosenObj = celObj;
                                }
                            }

                            if (objFound) {
                                Vec3f celObjPos = new Vec3f(chosenObj.getCelPos());
                                Vec3f cameraPos = add(celObjPos, new Vec3f(0, (float) chosenObj.getRadius() * 2, 0));

                                //If camera is locked to an object, then translating the camera will only
                                //do so with respect to that planet
                                //Hence, the camera is translated back to world coordinates by translating it
                                //the negative of its locked celObj position vector
                                if (camera.getLockedCelObj() != null) {
                                    cameraPos.translate(
                                            new Vec3f(
                                                    camera.getLockedCelObj().getCelPos()
                                            ).negate()
                                    );
                                }

                                //Make player 1 planet radius away from surface
                                camera.setPosition(multiply(worldunit_per_meters, cameraPos));
                                camera.setLookAt(multiply(worldunit_per_meters, celObjPos));

                                System.out.println("The camera position has been set to X= " + cameraPos.x + " Y= " + cameraPos.y + " Z= " + cameraPos.z);
                            } else
                                System.err.println(tokenNotFound);
                        } else
                            System.err.println(notEnoughInfo);
                        break;
                    default:
                        System.err.println(tokenNotFound);
                }
            } else
                System.err.println(notEnoughInfo);
            break;
        case "lock":
            if (tokens.length >= 2) {
                boolean objFound = false;
                CelObj chosenObj = null;
                for (CelObj celObj : physics.getCelObjs()) {
                    if (celObj.getName().toLowerCase().equals(tokens[1])) {
                        objFound = true;
                        chosenObj = celObj;
                    }
                }

                if (objFound) {
                    camera.setLockedCelObj(chosenObj);
                    camera.setPosition(new Vec3f(0, 0, 0));
                    System.out.println("The camera has been locked to " + chosenObj.getName());
                    System.out.println("Type 'unlock' to revert back to unlocked status");
                } else
                    System.err.println(tokenNotFound);
            } else
                System.err.println(notEnoughInfo);
            break;
        case "unlock":
            String celObjName = camera.getLockedCelObj().getName();
            //If camera is locked to an object, then translating the camera will only
            //do so with respect to that planet
            //Hence, the camera is translated back to world equivalent of where it is in
            //that celObj's space by translating it the celObj's position
            camera.setPosition(
                    add(
                            multiply(worldunit_per_meters,
                                    (new Vec3f(camera.getLockedCelObj().getCelPos()))),
                            camera.getPosition()
                    )
            );
            camera.setLockedCelObj(null);
            System.out.println("The camera has been unlocked from " + celObjName);
            Vec3f pos = camera.getPosition();
            System.out.println("The camera position has been set to X= " + pos.x + " Y= " + pos.y + " Z= " + pos.z);
            break;
        case "lookat":
            if (tokens.length >= 3) {
                switch (tokens[1]) {
                    case "celobj":
                        boolean objFound = false;
                        CelObj chosenObj = null;
                        for (CelObj celObj : physics.getCelObjs()) {
                            if (celObj.getName().toLowerCase().equals(tokens[2])) {
                                objFound = true;
                                chosenObj = celObj;
                            }
                        }

                        if (objFound) {
                            camera.setLookAt(new Vec3f(multiply(worldunit_per_meters, chosenObj.getCelPos())));
                            System.out.println("The camera is now looking at " + chosenObj.getName());
                        } else
                            System.err.println(tokenNotFound);
                        break;
                }
            } else
                System.err.println(notEnoughInfo);
            break;
        default:
            System.err.println(tokenNotFound);
    }
}
John Kugelman
  • 349,597
  • 67
  • 533
  • 578
Brendan M
  • 51
  • 1
  • 4

2 Answers2

0

Your instincts are right. The code you have could really benefit from being broken up somehow into smaller pieces. A great way to do this is to make it more data-driven. One way to encode a long list of commands is in a switch statement, but the problem is the statement grows longer and longer the more commands you have. A data-driven approach treats the command names and the code behind them as data, and separates the list of commands from the code that parses and executes the commands.

Let's start with a simple interface that represents a command handler. This is a function that is given a command's arguments and then does whatever the command does.

public interface CommandHandler {
    public void handle(List<String> arguments);
}

Then let's make the process() function data-driven. For now, let's tackle the first two commands, "close" and "toggle". We'll start simple, see if the idea makes sense, and then flesh out the implementation once we know at a high level what we want to do.

We'll create a map of command names to their handlers. This will give us a compact list of commands with the code behind each command split into separate callback functions. In case you're not familiar with it, Commands::close is a method reference. It gives us a CommandHandler object that calls the method Commands.close(), which we'll define later.

public static void process(String input) {
    Map<String, CommandHandler> commands = new HashMap<>();
    commands.put("close",  Commands::close);
    commands.put("toggle", Commands::toggle);

    List<String> tokens = Arrays.asList(input.toLowerCase().split("\\s+"));
    process(tokens, commands);
}

That looks pretty good. It's short and sweet. It splits the input string into tokens, but that's all the work it does. The rest is deferred to a second process() method. Let's write that now:

public static void process(List<String> tokens, Map<String, CommandHandler> commands) {
    String command = tokens.get(0);
    List<String> arguments = tokens.subList(1, tokens.size());

    CommandHandler handler = commands.get(command);

    if (handler != null) {
        handler.handle(arguments)
    }
}

That's the core of the command parsing logic. It looks up the command in the map, and executes the corresponding handler if it finds one. What's nice is that this method doesn't know anything about any particular commands. It's all very generic.

It's also setup to support sub-commands. Notice how it takes a list of tokens? And how it saves the arguments in a separate sub-list? Doing this means it can be called not just for top-level commands but also for sub-commands like "render".

The last piece of the puzzle is defining each of the command handlers. I've thrown them into their own class, but you don't have to do that. These can all be methods in your original class (I just don't know what you named it is all).

public class Commands {
    public static void close(List<String> arguments) {
        run = false;
    }    

    public static void toggle(List<String> arguments) {
        if (arguments.length == 0) {
            System.err.println(notEnoughInfo);
            return;
        }

        Map<String, CommandHandler> subCommands = new HashMap<>();

        subCommands.put("render", arguments -> {
            render = !render;
            System.out.println("Render setting set to " + render);
        });

        subCommands.put("physics", arguments -> {
            updatePhysics = !updatePhysics;
            System.out.println("Physics update setting set to " + updatePhysics);
        });

        subCommands.put("trails", arguments -> {
            showTrails = !showTrails;
            System.out.println("Show trails setting set to " + showTrails);
        });

        subCommands.put("constellations", arguments -> {
            showConstellations = !showConstellations;
            System.out.println("Show constellations setting set to " + showConstellations);
        });

        process(arguments, subCommands);
    }
}

toggle() shows off the sub-command parsing. Just like the code up top, it creates a map of sub-commands and registers their names and handlers. And just like up top, it calls the same process() function as before.

This time since the handlers are all so simple, there's no real need to break them out into separate named functions. We can use anonymous lambdas to register the handlers inline. Like Commands::close did earlier, arguments -> { code } creates a CommandHandler inline.

John Kugelman
  • 349,597
  • 67
  • 533
  • 578
0

consider using getopt

 Getopt g = new Getopt("testprog", argv, "ab:c::d");
 //
 int c;
 String arg;
 while ((c = g.getopt()) != -1)
   {
     switch(c)
       {
          case 'a':
          case 'd':
            System.out.print("You picked " + (char)c + "\n");
            break;
            //
          case 'b':
          case 'c':
            arg = g.getOptarg();
            System.out.print("You picked " + (char)c + 
                             " with an argument of " +
                             ((arg != null) ? arg : "null") + "\n");
            break;
            //
          case '?':
            break; // getopt() already printed an error
            //
          default:
            System.out.print("getopt() returned " + c + "\n");
       }
   }

related

Ray Tayek
  • 9,841
  • 8
  • 50
  • 90