Switch case with a String ?

  • (2 Pages)
  • +
  • 1
  • 2

21 Replies - 15976 Views - Last Post: 31 May 2011 - 02:39 PM Rate Topic: -----

#16 jon.kiparsky  Icon User is online

  • Pancakes!
  • member icon


Reputation: 7643
  • View blog
  • Posts: 12,890
  • Joined: 19-March 11

Re: Switch case with a String ?

Posted 30 May 2011 - 04:04 PM

If you insist on using a bunch of ifs, that's your privilege. In fact, if you really want to do it that way, I'm inclined to think you should do it that way - next time you run into this situation, you'll know exactly why it's not the best solution. :)

If you want to define the behavior you're looking to call as simply a bunch of methods, you would call them either from an icky-icky if-chain, or by some tricky reflection technique, which I think you're probably not going to be up to just yet.

The other solution would be the one proposed by nachtschatten:
actionMap.put("paint", new MyAction() {
  public void performAction() {
    //implement the paint-action here
  }
});


which I think is the best of the bunch proposed. Or, if anonymous classes are freaky for you, use his previous suggestion. Both are good, and neither is very difficult to understand.
Was This Post Helpful? 1
  • +
  • -

#17 baavgai  Icon User is offline

  • Dreaming Coder
  • member icon

Reputation: 5800
  • View blog
  • Posts: 12,636
  • Joined: 16-October 07

Re: Switch case with a String ?

Posted 30 May 2011 - 06:35 PM

View Postjon.kiparsky, on 30 May 2011 - 07:04 PM, said:

If you insist on using a bunch of ifs, that's your privilege.


If you like creating maps, interfaces, and a ton of boiler plate cut and paste for such a trival task, that's your privilege. It's certainly one way to do it, but let's just see what it takes. For our test case, let's look at three actions.
class ActionProcessor {
	interface MyAction { public void performAction(); }
	private Map<String, MyAction> actionMap;
	
	public ActionProcessor() {
		actionMap = new Hashtable<String, MyAction>();
		actionMap.put("paint", new MyAction() {
			public void performAction() {
				System.out.println("Painting now.");
			}
		});
		actionMap.put("repaint", new MyAction() {
			public void performAction() {
				System.out.println("Painting again.");
			}
		});
		actionMap.put("reset", new MyAction() {
			public void performAction() {
				System.out.println("Do over.");
			}
		});
	}

	public void processAction(String cmd) {
		cmd = cmd.toLowerCase();
		MyAction action = actionMap.get(cmd);
		if (action!=null) {
			action.performAction();
		} else {
			System.out.println("Unknown command.");
		}
		
	}
}



If you have a lot of commands, then maybe the map lookup will buy you something. The number would have to be in the thousands for you to notice. That's a whole lot of "put... new MyAction." At which point, you might reconsider your design entirely...

Compare to a simple self containted method:
class ActionProcessor {
	public void processAction(String cmd) {
		cmd = cmd.toLowerCase();
		if ("paint".equals(cmd)) {
			System.out.println("Painting now.");
		} else if ("repaint".equals(cmd)) {
			System.out.println("Painting again.");
		} else if ("reset".equals(cmd)) {
			System.out.println("Do over.");
		} else {
			System.out.println("Unknown command.");
		}
	}
}



Was is "best" depends on the actual case and is mostly subjective. Do I "know exactly why it's not the best solution"? No, I don't, not really. Sorry :P
Was This Post Helpful? 4
  • +
  • -

#18 jon.kiparsky  Icon User is online

  • Pancakes!
  • member icon


Reputation: 7643
  • View blog
  • Posts: 12,890
  • Joined: 19-March 11

Re: Switch case with a String ?

Posted 30 May 2011 - 07:43 PM

Okay, so what if you have actual code to execute? Obviously, if you're just determining what String to print, you'd just make a map of Strings - probably in a Properties file with all the other literal strings, loading them in your properties loader - and you'd be calling this:

System.out.println(stringMap.get(key));


If you insist on putting literals in the code, you'd have
stringMap.put("reset", "Do over.");
stringMap.put("paint", "Painting now");
stringMap.put("repaint", "Painting again");



But I'm assuming we're talking about actual executable code at the end of these decisions. So here are a few reasons why I get a slightly queasy feeling when I see this little toy class.

Clarity: Your if-based solution becomes spaghetti in a heartbeat, as soon as your successor on the project wants to extend this and puts a few sub-branches in the tree. Your action mapper (nice work, very pretty, by the way) segregates the work to be done quite neatly. It's still possible to convolute a particular method, but the damage is contained and localized. It's always very easy to follow the code path down to the actual code.

Structure: It is possible for your action mapper to become the God Class that we've all run into, but it's very unlikely, because anonymous inner classes don't really lend themselves to doing serious work. They're really good at firing messages, and that's about it. I've seen people use them to do heavy lifting, and it's always a marvel to me. However, your if-chain is only a series of easy steps from that nightmare. A maintenance coder adds a sub-branch, maybe you decide it's not really worth making a method call to just put a line into a textfield, then someone else changes it so that line depends on a little if or a ternary, and pretty soon you've got a codeblob - and nobody did anything really wrong to get it that way, they just did little things that were only wrong-ish.

Error-proofing: adding new branches or removing branches is a lot easier with your mapper. There are a lot more ways to get something that passes the compiler but does the wrong thing with a string of ifs and elsen than with inner classes - you can count on the inner classes to break if you miss a bracket, but you can easily get a dangling else when you string out an if-chain of any length. That, as you know, can be debugged, but it's a nuisance. And, of course, if your error involves a seldom-visited branch, you might never know until there's a strange bug. I'd rather have a method that fails fast.


How's that?

On the other hand, I think you might want to write up your action mapper as a tutorial - if your writing is as clear as your code, it'll be a big help!

This post has been edited by jon.kiparsky: 30 May 2011 - 07:44 PM

Was This Post Helpful? 2
  • +
  • -

#19 cfoley  Icon User is offline

  • Cabbage
  • member icon

Reputation: 1949
  • View blog
  • Posts: 4,049
  • Joined: 11-December 07

Re: Switch case with a String ?

Posted 31 May 2011 - 04:38 AM

Sometimes all you need is a few lines of if ... else if. Inventing a framework each time will often be using a sledgehammer to crack a nut. If statements, maps and pluggable actions are all great tools that fit different solutions. A good programmer will recognise when to apply each.
Was This Post Helpful? 0
  • +
  • -

#20 baavgai  Icon User is offline

  • Dreaming Coder
  • member icon

Reputation: 5800
  • View blog
  • Posts: 12,636
  • Joined: 16-October 07

Re: Switch case with a String ?

Posted 31 May 2011 - 06:31 AM

View Postjon.kiparsky, on 30 May 2011 - 10:43 PM, said:

Clarity: Your if-based solution becomes spaghetti in a heartbeat


Agreed, to an extent. The reason println is used in the example is to give the simplest possible code for testing. A larger example would call code, so:
if ("paint".equals(cmd)) {
	processPaint();
} else if ...



I agree with what you're saying. I also offer that not every problem is a giant enterprise level issue involing a scalablity framework and a UML diagram. Further, I don't imagine most posters are really in that headspace.

To be fair... how might I do this in the real world, for the big production thingy?

Something like this comes to mind.

interface CmdProcessor {
	static final int EXIT_SUCCESS = 0;
	static final int EXIT_FAIL = 1;
	boolean match(String cmd);
	int process(String cmd);
}

abstract class CmdProcessorImpl implements CmdProcessor {
	abstract public boolean match(String cmd);
	public int process(String cmd) { return match(cmd) ? processBody(cmd) : CmdProcessor.EXIT_FAIL; }
	abstract protected int processBody(String cmd);
}


abstract class SimpleCmdProcessor extends CmdProcessorImpl {
	protected String cmdMatch;
	public SimpleCmdProcessor(String cmdMatch) { this.cmdMatch = cmdMatch; }
	public boolean match(String cmd) { return cmdMatch.equals(cmd); }
	public int process(String cmd) { return match(cmd) ? processBody(cmd) : CmdProcessor.EXIT_FAIL; }
	abstract protected int processBody(String cmd);
}

class SwitchBoard implements CmdProcessor {
	protected List<CmdProcessor> list = new ArrayList<CmdProcessor>();
	public SwitchBoard() { }
	protected void addItem(CmdProcessor item) { this.list.add(item); }
	public boolean match(String cmd) {
		for (CmdProcessor item : list) {
			if (item.match(cmd)) { return true; }
		}
		return false;
	}
	public int process(String cmd) {
		for (CmdProcessor item : list) {
			if (item.match(cmd)) { 
				return item.process(cmd);
			}
		}
		return CmdProcessor.EXIT_FAIL;
	}
}

class MySwitchBoard extends SwitchBoard {
	public MySwitchBoard() {
		addItem(new SimpleCmdProcessor("abort") {
			protected int processBody(String cmd) { 
				System.out.println("ABORT!!!"); 
				return CmdProcessor.EXIT_SUCCESS;
			}
		});
		addItem(new CmdProcessorImpl() {
			public boolean match(String cmd) {
				return cmd.equals("paint") || cmd.equals("repaint");
			}
			protected int processBody(String cmd) { 
				System.out.println("Painting.");
				return CmdProcessor.EXIT_SUCCESS;
			}
		});
		addItem(new CmdProcessor() {
			public boolean match(String cmd) { return true; }
			public int process(String cmd) { 
				System.out.println("Unknown command: " + cmd);
				return -1;
			}
		});
	}
}


public class SwitchBoardTest {
	public static void main (String[] args) throws Exception {
		CmdProcessor cp = new MySwitchBoard();
		for(String cmd : new String[] { "abort","paint","foo" }) {
			System.out.println(cmd);
			cp.process(cmd);
			System.out.println();
		}
	}
}



I guess an Oberserver pattern and some kind of state might creep in. Then again, there are a whole lot of frameworks that have perfectly usable ways to deal with such things.

The reason I prefer the above approach to a dict mapper is flexibility. Sometimes you want to apply the same code to several possibilities. This design also allows the chaining of command processors, letting more complex actions be hooked in.

I've probably written several of these type of things over the years. No matter how well you plan, you'll get an odd case; it's the nature of frameworks. This one is pretty sturdy and doubtless benefits from that experience. It is why I'd prefer to keep code as simple as possible until such time as the design increases rather than limits the complexity. You simply can't plan for everything and trying to do so usually just gives you a beautiful mess.
Was This Post Helpful? 3
  • +
  • -

#21 jon.kiparsky  Icon User is online

  • Pancakes!
  • member icon


Reputation: 7643
  • View blog
  • Posts: 12,890
  • Joined: 19-March 11

Re: Switch case with a String ?

Posted 31 May 2011 - 12:15 PM

Quote

It is why I'd prefer to keep code as simple as possible until such time as the design increases rather than limits the complexity.


And from my perspective, it's usually simpler to do a thing in the most graceful manner possible from the start. You can't always predict what will happen with a project - that's one of the biggest cliches in the business! - but you can be pretty sure that if something is worth using, it'll expand. (or even if it isn't... sorry, I seem to have an emacs stuck in my throat, coff coff) I'd rather build for the assumption that the thing will grow. Maybe it's just that I don't find it so easy to use a switch or if-if-if construct. Obviously, they're not conceptually difficult, but I'm always interrupted by this stabbing pain in my eyes, and then I have to remove the pencils, wipe off the keyboard and screen, and start over.

So if I really think it's a one-off and it'll never be used again, I'll write it in perl. That's modification-proof, so you're safe.* But if it's worth writing it in Java, a basic rule for me is that if statements should be down in the lowest level of the code, in the core object definitions, and anything above that should be object-oriented, which means (among other things) that you're using message-passing as opposed to primitives and operators.

I realize that this is more about me and my preferences, but it's a practice that works pretty well for me. I don't keep to it strictly, but it makes me aware of when I'm writing C in Java, and that usually means I'm refactoring pretty soon.


*But even in perl, I typically use an array of function calls to handle input options, and if I wanted to allow the user to enter words instead of numbers, I'd use a map...
Was This Post Helpful? 2
  • +
  • -

#22 Renagado  Icon User is offline

  • D.I.C Regular
  • member icon

Reputation: 117
  • View blog
  • Posts: 388
  • Joined: 14-June 09

Re: Switch case with a String ?

Posted 31 May 2011 - 02:39 PM

baavgai and jon.kiparsky, nice discussion! Learned some things from it, thanks. Keep it coming ;)

I can agree with you both, but to be fair, I don't think enterprise-ready, maintenance-friendly kind of solutions are what TS(cornetto456) is after, so I made an example for him based on something KYA said, and switch enums. Don't get me wrong, I don't think long switches or if blocks are pretty, but as TS said, he just found out about switch so let's give him an example he can relate to.
    void processCommand(CommandName c)
    {
        switch (c)
        {
            case PAINT:
            {
                //some code
                break;
            }
            //etc
        }
    }

    enum CommandName
    {
        PAINT, PRINT, CANCEL
    }


Now you can change your code into something like:
if (evt.getKeyCode() == KeyEvent.VK_ENTER) {
	            processCommand(command)

And change your command from String into your newly created CommandName, et voila. And the nice thing is, you don't even need Java 7 for it.

This post has been edited by Renagado: 31 May 2011 - 02:42 PM

Was This Post Helpful? 1
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2