Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

This question already has an answer here:

We are using Java as a backend development language.

One year back, we wrote a method which uses switch cases based on Enums values. Since we are continuously adding enum members and according adding cases in the method, the method has grown to very large extent. Currently, we have around 100 enum fields and corresponding number of switch cases.e.g.

 class AClass{

   enum option{ o1, o2, o3...on}

   Value method(Option o){

      switch(o){
        case o1:
          value = deriveValue(p1,p2,p3);
        case o2:
          value = deriveValue(p2,p3,p4);
        .
        .
        .
        case on:
          value = deriveValue(p1,p2,p3);
      }
   } 
 }

Thus each time a business requirement comes, we add enum and corresponding switch case. Now the method has become too long and looks unmanageable, if we keep on adding the same logic in future.

To clean, we thought of replacing switch case with polymorphism by creating classes for the same, but again, we will end up creating n number of classes.

We are looking for small, simple and manageable solution.

----------------- Update ---------------------

As suggested, to elaborate more, Enum values are fields names for which a client needs a value. Thus if a client need the value of a new field, we add the field in enum, and the corresponding definition of how to fetch value for the newly added field, by adding the corresponding switch case.

In the switch case, we have a common reusable method e.g. deriveValue() (Please refer to the example given) to which we pass parameters required for deriving the value for the newly added field.

share|improve this question

marked as duplicate by gnat, Scant Roger, Matthieu M., Ixrec, Snowman Jan 5 at 23:28

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

11  
Could you describe in more detail what these enum values are and what sort of computations you're doing in each case? Right now, I simply don't know if there are any alternatives besides living with the switch statement or hiding it behind a class hierarchy; Kilian may be right in asserting there aren't any. – Ixrec Jan 5 at 7:41
5  
If each switch case just determines which parameters are passed to deriveValue method, then this determination can be encoded in some kind of data structure. Should be simple and manageable. – Euphoric Jan 5 at 7:45
1  
It really depends on what all the different options do. Hiding the large switch won't be a big improvement. Do they all do more or less the same thing, or are there different kinds of options? – RemcoGerlich Jan 5 at 9:24
3  
Those parameter lists (p1, p2, p3) smell like an object. The essence of that switch-case seems more like a function or even a static map [Option -> Parameter list] or some another kind of data structure like Euphoric suggested. So, I'm more interested of the arguments of the deliverValue-method. What's being passed here and why it's not an object? – COME FROM Jan 5 at 11:25
3  
I am afraid that the information provided is insufficient to answer the question. Specifically, without an exhaustive specification of what a case looks like (is it only ever a call to deriveValue? may the logic be completely different from one case to the other?) it is just impossible to give a more accurate answer than "it depends". – Matthieu M. Jan 5 at 18:13

What are you trying to achieve? Are you having difficulty understanding and maintaining the code structure you have, or are you just trying to follow some abstract idea of "good practice" that says methods should be short?

It looks to me as if the solution you have is quite adequate for the requirements you've been given. If there really are hundreds of similar computations that can be performed by a single line of code, then creating hundreds of subclasses with a single small method each is not necessarily better than simply listing all those parallel rules in one place. (The trouble with long methods is that they will become repetitive, inconsistent, and therefore confusing. But for listing a lot of parallel cases that have to be handled, a long list is perfectly fine.)

share|improve this answer
3  
A good example of this I can think of are CPU emulators, where the instructions to emulate are often in giant switch statements. – rhughes Jan 5 at 11:20
1  
I really want to give you a +1 for asking the all important question of "what are you trying to achieve?" I sort of want to give you a -1 for implying that switch is probably okay, because I've been on the receiving end of 500+ line switch statements, and I would like to reduce the number of times that happens in the world. The advice for short functions etc. is based on real experience and measurable productivity gains, not the whims of book writers. There are always exceptions--but those exceptions are exceptional. +0 on balance. – GrandOpener Jan 5 at 22:58

You may create a table. The first column will be the switch value and the other columns will be the arguments passed on the deriveValue method. For example

table OPTIONS
OPTION -  ARGUMENT1 -  ARGUMENT2 -  ARGUMENT3
o1          p1          p2          p3
o2          p2          p3          p4
on          p1          p2          p3

You can then write one general function that uses the above table to produce the appropriate function call based on the option.

share|improve this answer
3  
Assuming the OP didn't overly simplify the method in question, this is definitely the way to go. When the business rules get updated, you just update a configuration file. No need to even recompile. – Hoten Jan 5 at 15:47

You could use abstract methods in the enum, thereby having better code locality (for better legibility and maintenance, not talking performance here).

class AClass{
    enum Option{
        o1 {
            @Override
            Value deriveValue() {
                return deriveValue(p1, p2, p3);
            }
        }, ... on {
            @Override
            Value deriveValue() {
                return deriveValue(p1, p2, p3);
            }
        };
        abstract Value deriveValue();
    }

    Value method(Option o){
        return o.deriveValue();
    }
}
share|improve this answer
    
Please explain what is "code locality" and "perf". – Tulains Córdova Jan 5 at 20:19
2  
@user61852 locality, here, refers to keeping the logic for calculating the derived value and the symbol definition in the same place; you don't need to visit two separate areas to add/remove/modify a value. perf is simply an abbreviation for performance; the code won't run as fast as a switch statement, although for normal implementations of business logic, this probably won't matter. – phyrfox Jan 5 at 20:35
    
@phyrfox I wouldn't have said it better :-) – cadrian Jan 6 at 6:19

It's worth noting that, if you have this kind of situation where there's only one single function in the system always which will perform such checks, and it does not have any need whatsoever to be open for external extension, then throwing a polymorphic solution at it may actually leave you finding that your daily maintenance costs have actually increased rather than decreased.

Here, excluding a need for open extension, the practical difference between polymorphic and conditionals might boil down to whether you prefer more centralized, unstable code or more decentralized, stable code.

More decentralized, stable code might actually be favorable in some cases if it leads to less version control toe-stepping, e.g. More centralized, unstable code here would tend to be shorter, if only as a result of avoiding the boilerplate associated with defining new classes for each little simple case.

Currently, we have around 100 enum fields and corresponding number of switch cases.e.g. Thus each time a business requirement comes, we add enum and corresponding switch case.

This feels like the problem to me if we can avoid it. Is there no kind of categorization that can accompany so many enum fields? With the kind of ordinal example you gave, that wouldn't find much meaningful categorization. I'm hoping it was just an analogical one.

If you can find some kind of meaningful categorization to organize these enum fields, you can potentially use more than one function. Each function (still pretty coarse) could handle a range of cases, with an outer function checking which function to call based on the range.

That's a simple and not-so-intrusive solution, provided you can find some sort of categorization, to still have a fairly centralized and simple solution, but leave behind stable functions for the range of options you've already handled. It'll also reduce the instability of the function implementations involved, since the fewer range of options they handle, the fewer reasons they'll find to individually change.

You can also potentially use look-up table solutions possibly without a subtype involved for all 100 options, which gives you one centralized place to modify but possibly with a possibly more-preferable syntax. It depends on the nature of p1, p2, ..., pn. For example, if they are homogeneous, you might be able to use an array and make a LUT based on the index ordering with option enum fields modeling an index into the LUT. Example (pseudocode):

lut[o1] = {0, 1, 2};
lut[o2] = {1, 2, 3};
...
lut[on] = {0, 1, 2};

Value method(Option o){
    int i1 = lut[o][0];
    int i2 = lut[o][1];
    int i3 = lut[o][2];
    deriveValue(p[i1], p[i2], p[i3]);
}

This doesn't really do much architecturally (though it is open for extension if you allow lut entries to be added from the outside) but you might prefer the syntax. It applies if your solution wasn't a remote analogy and actually just calls deriveValue based on the option with a different ordering of arguments (in which case I'd think there would be at least more than 3 parameters in actuality to call for so many options).

share|improve this answer
    
“less centralized, unstable code or more decentralized, stable code” — aren’t “less centralized” and “more decentralized” the same thing? – Paul D. Waite Jan 5 at 12:41
1  
@PaulD.Waite Argh, apologies! I goofed and put "less" instead of "more" there. – Ike Jan 5 at 12:45

You could apply the general strategy used to deal with big numbers -- structure. Perhaps the prolific number of options handled uniformly is a sign of too little hierarchy? Perhaps the cases fall neatly in groups which seem to belong together (like I/O, manipulation, computation etc.)? (Perhap not -- then please ignore ;-) ).

This could be reflected with a a hierarchical enum system. One enum characterizes the group, the next one the specific operation.

The groups could be coded separately and would each be more managable. Having them in different source files/classes also makes editing errors in unrelated places less likely, may speed up compilation and should reduce dependencies (from system libraries as well as from other custom classes).

Of course this approach is also conceivable as a design idea for polymorhpism: One could have specific option/action classes deriving from group classes deriving from a generic option class), with an opportunity to bundle common code further up in the hierarchy.

share|improve this answer

Changing to classes won't help you. In terms of clarity, robustness, compactness, maintainability and performance it doesn't get any better than enums and a switch statement. What you should be asking yourself is if you are dealing with types at all. Considering that you are frequently and continuously adding enums, it seems to me you are dealing with data, not types. You might need a single class and multiple instances. This is what DesignerAnalyst implies with the table proposal. You may still hard-code your input data for your object instances or slap the input in a text file, the big question remains: what ARE those enum values really? How do they classify?

share|improve this answer

Adding more and more classes is not really a problem, if they are parts of a well defined structure that allows you to ignore them when you want to: just put them in a module, and only access the module when you have to modify it.

IMHO, each option represents a way to compute a value, therefore in OO, each opinion should be represented by an object having a specific implementation of deriveValue(). In Java, that means that each object must have a specific class defining the implementation. So your code becomes something like:

In your option package:

public interface Option {
     Value deriveValue(? p1, ? p2, ? p3);
}

public FirstOption implements Option {
    Value deriveValue(? p1, ? p2, ? p3){ ... } 
}

In an other package;

class AClass{
   private Option o;

   Value method(){ o.deriveValue(p1,p2,p3); }
}

If you have different behaviors to specify, each of them can be represented by a single call instead of a big switch. Caveat: each time you want to add a new behavior, you have to open each option class to add a new method.

share|improve this answer
    
The thing is, the exact same argument would also work for "just keep the big switch". As long as it's easy to understand and extend, there's nothing inherently wrong with a big switch. It might be better to replace it with class polymorphism, but it might not. – Luaan Jan 5 at 11:02
    
The problem is that the ability to maintain the switch will decrease as the number of cases increases: you are getting a longer method each time a new option is inserted. Conversely, the method() body will have the same length and complexity, independently of the number of available options. And each option manages a single, isolated, and therefore more easily maintainable behavior. Plus, again, a switch places a option-based behavior in something that is not an option, but has or receive an option. – mgoeminne Jan 5 at 11:15
    
That doesn't really matter much when the method only does that one single thing - routing. It's pretty much equivalent to a lookup table between the enum and a function. As long as you keep it simple like this, it doesn't really matter how many lines the method has - the complexity doesn't really increase at all. Of course, in practice, your solution is even more equivalent to that lookup table, especially when you include that the OP will still have to have a mapping from enums to your options classes - which will probably either be another giant switch or a lookup table :) – Luaan Jan 5 at 13:56

You have more and more options, leading to more and more enum cases, and more and more code implementing something for each option. You can try to do something about the growing number of options - well, maybe you can't.

If you can't prevent the number of options from growing, then a switch statement is just as good as any other method to choose between the different options. And it looks like very little additional code is needed beyond the actual code to evaluate each option. A decent compiler will tell you about missing cases.

If you don't like this advice: One day you reach the point where you worry about simple and maintainable code, and not about violations of some abstract principle that you just heard about at your latest course. Rules are there to be broken unless you are a novice. Now if instead of "this is a violation of the open-closed principle" you could actually give a practical example...

share|improve this answer
5  
You also have to take into account the fact that enum+switch structures tend to make you option-based logic spread all over the code, instead of concentrate it in a single point. It's a violation of the open-closed principle. – mgoeminne Jan 5 at 10:02

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