Strict Standards: Redefining already defined constructor for class WP_Http in /var/www/serverdude.dk/public_html/wp-includes/http.php on line 61How to refactor a refactored switch/case statement « ServerdudeDeprecated: preg_replace() [function.preg-replace]: The /e modifier is deprecated, use preg_replace_callback instead in /var/www/serverdude.dk/public_html/wp-includes/kses.php on line 1002

Deprecated: preg_replace() [function.preg-replace]: The /e modifier is deprecated, use preg_replace_callback instead in /var/www/serverdude.dk/public_html/wp-includes/kses.php on line 1003

So I read through this - I know I dislike switch/case jump tables, though not as much as I hate if-else-if - or as I like to reminisce Sid Meier’s Pirates! and call it the “evil El Sif”

Gianluca is quite right, that one option would be to use the Strategy pattern, but then goes on to show how not to implement this pattern by adding a method for each of the enums, then tie a specific implementation inside the enum ending up with a less readable and less maintainable code.

The enum part is right - eliminate the magic strings, define the different types.

The strategy interface definition is wrong - the name “HasStrategies” does not convey any useful information. The 2 methods bind concrete enums to an interface, 1 abstract method, e.g. ‘execute’ should be sufficient. Then the specific strategy is pushed inside the enums themselves. Enums should not care for whichever strategies you have for them, thus that sort of coupling is not wanted.

In the Decider class, we now define the specific strategy to use, which sort of defies the purpose of extracting the code from a switch - the specific class will now have 2 reasons for change:

If we add another value to the enums, then we need to change the Decider implementation as well, that is contrary to the Open Close Principle. From the looks of it, we have to change the enums (well, that’s a given), the strategy, and the decider implementation.

What I’d recommend:

Define the strategy interface using only one method

interface Strategy {
String execute();
}

Simply define the values

enum Values {
PIPPO, PLUTO;
}

Implement the strategies for each of the values, and add them to an EnumMap

Well, the mapping from a primitive to the enum should not take place inside the method, the Decider interface should be modified to fix such hacks, if the String, which, is null or does not represent a Value, then a NullPointerException and IllegalArgumentException respectively will be thrown from the Value conversion.

The names are still not meaningful.

With this solution a new enum value will require a change to Values and the implementation for its strategy inside the ValueStrategies.

If re-use of the strategy implementations were of concern, then naturally they should be implemented in their own classes and not as anonymous values inside the map.

This entry was posted
on torsdag, november 26th, 2015 at 01:40 and is filed under Software development, programming.
You can follow any responses to this entry through the RSS 2.0 feed.
Both comments and pings are currently closed.