1

In the codebase I'm currently working on, it's common to have to take a string passed in from further up the chain and use it as a key to find a different String. The current standard idiom is to use switch statements, however for larger switch statements (think ~20-30 cases) sonarqube says it's a code smell and should be reduced for cyclomatic complexity. My current solution is to use a static HashMap, like so

private static final HashMap<String, String> sortMap;
static {
    sortMap = new HashMap<>();
    sortMap.put("foo1", "bar1");
    sortMap.put("foo2", "bar2");
    sortMap.put("foo3", "bar3");
    etc...
}

protected String mapSortKey(String key) {

    return sortMap.get(key);
}

However this doesn't seem to actually be any cleaner, and if anything seems more confusing for maintainers. Is there a better way to solve this? Or should sonarqube be ignored in this situation? I am aware of using polymorphism, i.e. Ways to eliminate switch in code, however that seems like it is overkill for this problem, as the switch statements are being used as makeshift data structures rather than as rudimentary polymorphism. Other similar questions I've found about reducing switch case cyclomatic complexity aren't really applicable in this instance.

JCD
  • 13
  • 1
  • 5
  • 1
    Can't you outsource the mappings into a file and read it in from there? That approach is far more dynamic. Better than to have 100 switch-case statements or map-puts hardcoded into your program. Besides that I think the map-approach is far more clean. But I don't see the reason why you would need to use *static-blocks* here, I don't like them... – Zabuzard Oct 17 '17 at 20:12
  • https://blog.sonarsource.com/cognitive-complexity-because-testability-understandability says "even McCabe acknowledged in his original paper that the treatment of case statements in a switch didn't seem quite right", with regards to cyclomatic complexity. So you could decide to ignore it. – Kayaman Oct 17 '17 at 20:12
  • @zero298 I feel like using polymorphism isn't the right way to solve this, as the way the switch statements are being used now is as a rudimentary data structure, rather than a way of determining what to run. – JCD Oct 17 '17 at 20:17
  • @Kayaman I agree that it is largely an arbitrary metric, but management is on a big technical debt push, and as a junior developer a lot of the more minor code smell stuff gets pushed down the stack to me. I could probably ignore it, but if theres a better way I would prefer to do so. – JCD Oct 17 '17 at 20:19
  • There's no *general* better way, it depends on your actual code. – Kayaman Oct 17 '17 at 20:25
  • @Kayaman Thats a good point. I think I'll go with what Zabuza said and just use non static maps since the map shouldn't ever need to be changed once it's actually set. Thanks everyone for the help. – JCD Oct 17 '17 at 20:34

1 Answers1

0

If, by your example, this is just the case of choosing a mapped value from a key, a table or properties file would be a more appropriate way to handle this.

If you're talking about logic within the different switch statements, you might find that a rules engine would suit better.

You hit upon the major requirement: maintainability. If we are coding in too much logic or too much data, we have made brittle code. Choose a design pattern suited to the type of switched information and export the functionality into a maintainable place for whomever must make changes later... because with a long list like this, chances are high that changes will be occurring with some frequency.

J E Carter II
  • 1,436
  • 1
  • 22
  • 39