22 public class CrapiolaManager : ICrapiolaManager
23 {
24 public void DoSomething(SomeEnumOfTypesWtf someType)
25 {
26 if (someType == SomeEnumOfTypesWtf.SomeType1)
27 {
28 //1. Do some work and calculations etc and more crap code...
29 }
30 if (someType == SomeEnumOfTypesWtf.SomeType2)
31 {
32 //2. Do some work and calculations etc and more crap code...
33 }
34 if (someType == SomeEnumOfTypesWtf.SomeType3)
35 {
36 //3. Do some work and calculations etc and more crap code...
37 }
38 if (someType == SomeEnumOfTypesWtf.SomeType4)
39 {
40 //4. Do some work and calculations etc and more crap code...
41 }
42 if (someType == SomeEnumOfTypesWtf.SomeType5)
43 {
44 //5. Do some work and calculations etc and more crap code...
45 }
46
47 throw new InvalidOperationException("Invalid type passed");
48 }
49 }
- Lots of bugs/issues!!!
- New stories take longer to do!!!
- Its duplication of code!
- It’s not easy to Test!!
- Not easy to read when you have to follow code through so many layers, you want a piece of logic in ONE place… Domain Model Entity?
- It’s a problem when a new type appears! Somehow the person working on the new type needs to magically pull out of his ass some knowledge to understand where all the switch’s and if’s are in the code to alter them to add a new case! Hmmm… I wonder how many of these if’s and switch’s get missed when adding a new type? Oooo let me guess… ALL OF THEM! It’s completely unmaintainable rubbish!
What’s it there for, are you going to create another manager from the same interface(highly unlikely)? Is it there for the Dependency Injection instead? Then I am sure you can inject it without an interface. Even if you’re going down this road asking yourself these questions, you may realise 'ooo' the code is in the wrong place and we shouldn’t have Managers and Helpers littered everywhere! And this is another post all in itself (Say NO to managers/Helpers)
Throwing an exception if the type doesn’t exists is also crap!
I mean seriously if you were testing this and for some reason you never hit this if/switch statement after adding your new type, that exception will never be thrown!! Oh yes and I guarantee it will be thrown when a user uses it!
- BIG Problem!
- Support costs!
- Customer trust levels!
- The list carries on…
A suggestion to resolve this problem! (Not too much detail here)
(This is the basics of the start of the fix but should pretty much be an answer for this issue). Hmmm… how about use an interface or a base class (depending on what you require) and derive these types from that interface/base class to force implementation of the method that does the work in them dumb ass if’s and switch’s! It takes 2 minutes to write, and NOT 2 days to fix bugs when adding new types!
22 public interface IDoSomething
23 {
24 void DoSomething();
25 }
26
27 public class DoSomehting1 : IDoSomething
28 {
29 public void DoSomething()
30 {
31 //1. Do some work and calculations etc and more crap code...
32 }
33 }
34 public class DoSomehting2 : IDoSomething
35 {
36 public void DoSomething()
37 {
38 //2. Do some work and calculations etc and more crap code...
39 }
40 }
41 public class DoSomehting3 : IDoSomething
42 {
43 public void DoSomething()
44 {
45 //3. Do some work and calculations etc and more crap code...
46 }
47 }
48 ...
49 public class DoTheWork
50 {
51 public void Work(IDoSomething doSomething)
52 {
53 //The object does the work not some external object that would be a nightmare to maintain.
54 doSomething.DoSomething();
55
56 //The above is called instead of doing...
57 //ICrapiolaManager someManager;
58 //someManager.DoSomething();
59 }
60 }
It’s not all about finishing as fast as possible and just letting your fingers typing your brain farts for 100% of the day, surprisingly it’s a very high amount of THINKING! The chances are when you come to the solution in your head after thinking around all the issues, scenarios and solutions to the problem you will find you will hardly type any code!
Here is a link I found very quickly (I am not sure who it is by) on the issue there is a lot of info here and was hoping to find something by the main man himself Martin Fowler who we should all know and love! It pretty much goes over a lot of issues with using a switch (or ifs in our case from this code, it’s the same thing) in an OO language. IMO we should be able to turn the ‘switch’ and ‘if’ statements OFF in the compiler and to be only allowed to turn them on where we need them, more than likely in the Domain Model Entities only!
I can’t stress how important something as simple as this is, It’s a simple piece of code and if thought about for at least 30 seconds, the problem could be solved much more elegantly and safely. If you’re unsure on something don’t hesitate to ask the question to a colleague. I don’t have all the answers but I can see some of them. Just by asking the question sometimes you actually answer it yourself without hearing an answer (I do that a LOT).
If this doesn’t seem to be an important issue to you and want to continue to switch and if on types I am sure there are plenty of job opportunities out there as a farmer! Let us developers write robust code and not fix someone else's problems all the time.
