{"id":315,"date":"2015-11-26T01:40:23","date_gmt":"2015-11-26T00:40:23","guid":{"rendered":"http:\/\/serverdude.dk\/?p=315"},"modified":"2015-11-26T01:40:23","modified_gmt":"2015-11-26T00:40:23","slug":"how-to-refactor-a-refactored-switchcase-statement","status":"publish","type":"post","link":"https:\/\/serverdude.dk\/?p=315","title":{"rendered":"How to refactor a refactored switch\/case statement"},"content":{"rendered":"

When good intentions go slightly wrong<\/p>\n

For some odd reason I picked up a link to DZONE on “How to refactor a switch\/case statement” – the link https:\/\/dzone.com\/articles\/how-to-refactor-a-switchcase-statement<\/a> is now defunct, I’m not sure why. Anyway, Gianluca Tomasino, the original author still has the article on his blog<\/a>.<\/p>\n

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”<\/p>\n

Gianluca is quite right, that one option would be to use the Strategy pattern<\/a>, 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.<\/p>\n

The enum part is right – eliminate the magic strings, define the different types.<\/p>\n

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.<\/p>\n

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:<\/p>\n

    \n
  1. Change to the strategy<\/li>\n
  2. Change to the enum definitions<\/li>\n<\/ol>\n

    “A class should have one, and only one reason to change.” That is the intent of the Single Responsibility Principle<\/a><\/p>\n

    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<\/a>. From the looks of it, we have to change the enums (well, that’s a given), the strategy, and the decider implementation.<\/p>\n

    What I’d recommend:<\/p>\n

    Define the strategy interface using only one method<\/p>\n

    interface <\/span>Strategy {\r\n    String execute();\r\n}<\/pre>\n
    Simply define the values<\/p>\n
    enum <\/span>Values {\r\n    PIPPO<\/span>, PLUTO<\/span>;\r\n}<\/pre>\n
    Implement the strategies for each of the values, and add them to an EnumMap<\/p>\n
    class <\/span>ValueStrategies {\r\n    final static <\/span>EnumMap<Values, Strategy> MAP <\/span>=\u00c2\u00a0\r\n             new <\/span>EnumMap<Values, Strategy>(Values.class<\/span>);<\/pre>\n
        static <\/span>{\r\n        MAP<\/span>.put(Values.PIPPO<\/span>, new <\/span>Strategy() {\r\n            @Override\r\n<\/span>            <\/span>public <\/span>String execute() {\r\n                return <\/span>\"methodA\"<\/span>;\r\n            }\r\n        });\r\n        MAP<\/span>.put(Values.PLUTO<\/span>, new <\/span>Strategy() {\r\n            @Override\r\n<\/span>            <\/span>public <\/span>String execute() {\r\n                return <\/span>\"methodB\"<\/span>;\r\n            }\r\n        });\r\n    }\r\n    static <\/span>Strategy get(Values value) {\r\n        return <\/span>MAP<\/span>.get(value);\r\n    }\r\n}<\/pre>\n
    Implement the decider using these elements:<\/p>\n
    public class <\/span>AltDecider implements <\/span>Decider {\r\n\r\n    @Override\r\n<\/span>    <\/span>public <\/span>String call(String which) {\r\n        Values value = Values.valueOf<\/span>(which.toUpperCase());\r\n        return <\/span>ValueStrategies.get<\/span>(value).execute();\r\n    }\r\n\r\n}<\/pre>\n
    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.<\/p>\n
    The names are still not meaningful.<\/p>\n
    With this solution a new enum value will require a change to Values and the implementation for its strategy inside the ValueStrategies.<\/p>\n
    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.<\/p>\n","protected":false},"excerpt":{"rendered":"
    When good intentions go slightly wrong For some odd reason I picked up a link to DZONE on “How to refactor a switch\/case statement” – the link https:\/\/dzone.com\/articles\/how-to-refactor-a-switchcase-statement is now defunct, I’m not sure why. Anyway, Gianluca Tomasino, the original author still has the article on his blog. So I read through this – I […]<\/p>\n","protected":false},"author":1,"featured_media":0,"comment_status":"open","ping_status":"open","sticky":false,"template":"","format":"standard","meta":{"footnotes":""},"categories":[6,10],"tags":[],"class_list":["post-315","post","type-post","status-publish","format-standard","hentry","category-programming","category-software-development"],"_links":{"self":[{"href":"https:\/\/serverdude.dk\/index.php?rest_route=\/wp\/v2\/posts\/315","targetHints":{"allow":["GET"]}}],"collection":[{"href":"https:\/\/serverdude.dk\/index.php?rest_route=\/wp\/v2\/posts"}],"about":[{"href":"https:\/\/serverdude.dk\/index.php?rest_route=\/wp\/v2\/types\/post"}],"author":[{"embeddable":true,"href":"https:\/\/serverdude.dk\/index.php?rest_route=\/wp\/v2\/users\/1"}],"replies":[{"embeddable":true,"href":"https:\/\/serverdude.dk\/index.php?rest_route=%2Fwp%2Fv2%2Fcomments&post=315"}],"version-history":[{"count":4,"href":"https:\/\/serverdude.dk\/index.php?rest_route=\/wp\/v2\/posts\/315\/revisions"}],"predecessor-version":[{"id":319,"href":"https:\/\/serverdude.dk\/index.php?rest_route=\/wp\/v2\/posts\/315\/revisions\/319"}],"wp:attachment":[{"href":"https:\/\/serverdude.dk\/index.php?rest_route=%2Fwp%2Fv2%2Fmedia&parent=315"}],"wp:term":[{"taxonomy":"category","embeddable":true,"href":"https:\/\/serverdude.dk\/index.php?rest_route=%2Fwp%2Fv2%2Fcategories&post=315"},{"taxonomy":"post_tag","embeddable":true,"href":"https:\/\/serverdude.dk\/index.php?rest_route=%2Fwp%2Fv2%2Ftags&post=315"}],"curies":[{"name":"wp","href":"https:\/\/api.w.org\/{rel}","templated":true}]}}