The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

Open Closed Principle and Switch statements

Hi there,

I've been reading Robert C Martins book and trawling the internet regarding the switch statement and how if we all keep using it, the world will come to an abrupt halt.  I need some help here in understanding how you get around some of the issues regarding the decision logic if you dont have it, or if I have got the wrong end of the stick here...

If we had 3 products (A,B and C) and we receive a string to indicate which product we were interested in, is it ok to move this to a factory object which creates us a product based on this string e.g.

Product ProductFactory.CreateProduct(string ProductType)
{
  Switch(ProductType)
  case "A" return new ProductA
  case "B" return new ProductB
  blah...
}

or is there some other way that I am missing here?

Thanks
Andrew Send private email
Wednesday, August 27, 2008
 
 
The switch-statement is beautiful.

If there is a lot of products, maybe storing it in a hash map?
Tov Are Jacobsen Send private email
Wednesday, August 27, 2008
 
 
I've just received the book today, so I haven't got to that bit yet, but that's exactly the place for switch statements, unless you add functions into a map and get the function out with the string parameter and call the function to get a new object.

Anyway you want that function closed wrt adding new classes.
A Send private email
Wednesday, August 27, 2008
 
 
Can you use reflection to instantiate a Product based on its type? That way if new Products come along you don't have to add another case to the switch statement.
John Topley Send private email
Wednesday, August 27, 2008
 
 
Every decade, people like to say the everything we thought we were doing right the previous decade was actually a horrible way to program.

If Robert Martin were really that great, he's program the next Big Thing and become a billionaire, instead of writing his books.
Me too
Wednesday, August 27, 2008
 
 
I personally think you should replace your product type string with an enumeration. After that, having the select case structure in your factory is fine. However, if you have a lot of select-case logic around product type in other places, then you should consider think hard about your design.

The problem with lots of select-case logic is if you add another "case," you can't easily guarantee that every relevant decision tree in your application has been updated. Hiding those choices in some central-use class such as a Factory or Validation engine gets around that problem.

Oh, and Robert C. Martin is awesome. :-)
Happy Coding!
Chris McKenzie Send private email
Wednesday, August 27, 2008
 
 
from his website:
"process improvement consulting, object-oriented software design consulting , training, and development services "

So yeah if he isn't telling you what you are doing sucks how is he supposed to sell you the improvement?
Brian
Wednesday, August 27, 2008
 
 
Can someone tell me how the switch statement is going to bring about Armageddon?
Matt B
Wednesday, August 27, 2008
 
 
I think the point is that every time a new Product is introduced, you have to add another case to the switch statement.
John Topley Send private email
Wednesday, August 27, 2008
 
 
You are substituting scalability for simplicity. I see nothing wrong with using a simple switch statement as you've done. The alternative easily becomes the configuration disaster that the typical Java provider pattern uses. In order to directly instantiate the class without a switch statement the user must enter the fully qualified class name into some sort of data source. This is significantly harder for them and doesn't add much value because you really can't edit/validate it for them. Once you try to validate it you've lost the benefits of not using a switch statement and you are back to square one.

For example, if I have two possible choices for logging support "File" and "MessageQueue" then I can easily validate that the passed value is one of those two choices and can inform the user of what the possible values are if they pass in something else. But if the user has to pass in a fully qualifed class name: "org.mycompany.logging.FileLogger" vs. "org.mycompany.logging.MessageQueueLogger" then I can't really do anything if they mispell the value or enter something that isn't supported. And don't even get me started on which value is more user friendly to enter!

If the number of possible combinations doesn't change very often then it is easier and more user friendly to just use the switch statement. If the number of combinations is large or unknown then using something like the provider pattern makes more sense. In 90+% of the cases I've run into over the years a switch statement works just fine. There are typically only a couple of options available and adding new options always involves some sort of coding effort anyway. The least of your worries will be adding one more line to an existing switch statement.
dood mcdoogle
Thursday, August 28, 2008
 
 
dood,

"In order to directly instantiate the class without a switch statement the user must enter the fully qualified class name into some sort of data source."

I don't think that follows, nor does it follow that you can't do any input validation. It's been a while since I've done any Java, but in C# at least, it's easy to take an unqualified class name, and find the fully-qualified classname.

It's also really easy to use Reflection to determine whether a particular class descends from the right superclass and/or implements the right methods.

In a project I'm currently working on, we use this sort of construct in a couple of places, and it really is more flexible and ultimately easier to maintain than a switch statement, once you get over a certain size.

Having said that, the more interesting cases are not where you have a switch statement that creates new objects of various classes, that's fairly defensible.

What really sets off alarm bells for me is when you have code that performs a switch statement over some variable, then performs casts on another object based on the value of the switch. That really is reinventing the dispatch mechanism.
Mark Bessey Send private email
Thursday, August 28, 2008
 
 
"I don't think that follows, nor does it follow that you can't do any input validation. It's been a while since I've done any Java, but in C# at least, it's easy to take an unqualified class name, and find the fully-qualified classname. "

That's true. I can constrain the problem such that only the class name without the fully assembly path is required. But I would be requiring the class to be in a specific assembly or set of assemblies for my search. And as you said I can verify that they class being created is a subclass of the correct type (or implements the correct interface). But again, that is much harder than simply having a simple switch statement. My point was that you are substituting scalability with simplicity. I also believe that it is much easier on the user to be able to select from a drop down of options than to have to remember a class name (even if it doesn't have to be fully qualified with the assembly name and namespace). I'm sure that you will come back and tell me that I could use reflection to get the list of valid class names and populate a drop down as well. But again, although it's possible it isn't nearly as easy and I'd have to constrain the problem somehow so that I am not searching the entire machine for assemblies that might contain classes that match the required criteria.

I agree that the factory method case is a simple one. I think where switch statements really come in handy is when you are dealing with an abstract concept that is not encapsulated in a single class. Using my previous example, what if you needed to write some special business logic code that acted differently based on the choice of "File" vs. "MessageQueue"? Having a hard coded constant string in your application makes this much easier than needing to know what specific class would implement the functionality. Not that it can't be done with classes but it is again much easier with switch statements.

I'll end with pointing out as others have that there is no one proper technique. We actually use both techniques. But when we are looking at having the user configure the system from a small list of choices then we usually opt for the simple switch statement. It is very rare that you actually end up going back and adding several more choices so the maintenance cost is very low as is the initial design cost. We tend to use the provider pattern approach with class names when the programmer is responsible for configuring the functionality on a customer by customer basis and once its done you never touch it again. For example, each customer's specific abstract factory class is created from a fully qualified class name in the application.config file. This is something that is done once by the programmer and then checked into the repository. It never changes and is a design time option.
dood mcdoogle
Thursday, August 28, 2008
 
 
From Robert Martin's new book "Clean Code":

"My general rule for switch statements is that they can be tolerated if they appear only once, are used to create polymorphic objects, and are hidden behind an inheritance relationship so that the rest of the system can't see them.  Of course every circumstance is unique, and there are times when I violate one or more parts of that rule."

So it seems like your product example is okay.
you're okay
Sunday, August 31, 2008
 
 
I use that factory method switch statement all the time and there is absolutely nothing wrong with it.

In my opinion, you have to think with the information you have and figure out if there are side effects, maintainability issues and then also recognize that you are looking really for optimum code, not necessarily perfect by-all-the-rules code.  In other words, are there tradeoffs to consider and can you live with them?  So here you have an arbitrary rule that switch statements are bad, so don't use them.  Well that's sort of silly.  The criticism about switch statements seems to be that they can get too long.  Well trade that off with using other methods, which would undoubtedly take longer to code and the benefit is really, having a few lines of code rather than one long switch statement.  However, it is easy to figure out what is going on with the switch statement, and once you've written the factory method, it is so simple to comprehend that you are probably never going to have to look at that code again, so who cares whether there is a switch statement in it?
Mike S Send private email
Sunday, August 31, 2008
 
 
I think the idea of switch statements are bad came from Fowler's Refactoring, where it is listed explicitly as a code smell.  But what he says about them is what's important:

"The problem with switch statements is essentially that of duplication.  Often, you find the same switch statement scattered about a program in different places.  If you add a new clause to the switch, you have to find all these switch statements and change them."

Your example does not suffer from this.
Mike S Send private email
Sunday, August 31, 2008
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz