“Code smell, also known as bad smell, in computer programming code, refers to any symptom in the source code of a program that possibly indicates a deeper problem”
Wikipedia
In the previous part we started finding out what code smells are and how to find them by some symptoms before they make your application go wrong. The next bunch of “smells” in common represents different problems in class structure and classes relation. So, let’s start.
Type Embedded in Name
Description: You put redundant information to your methods’ names. For instance, instead of using call method with add() name, you use addInteger(), addConcreteTypeOfUser().
Problems: Having done that you eliminate polymorphism and make method absolutely inable to receive another types of input parameters. It makes you do changes in your method’s name if the type was changed.
Decisions:
– rename method with more general name, try to avoid putting extra information about it and focus on that name that reflects exactly what the method does (no more or no less). A method should only have a single purpose. If your method contains too much functionality, then you should split it into more than one method.
Speculative Generality
Description: You write a lot of code to resolve future problems, your mighty hierarchy of abstract classes predicts everything, but it creates a bunch of problems with maintenance at the current life stage of project.
Problems: You over-generalize your code attempting to predict future needs. It’s always harder to understand and to maintain the resulting classes than to handle things which are actually required.
Decisions:
– write code to solve today’s problems, and worry about tomorrow’s problems when they actually materialize. Remember about YAGNI (You ain’t gonna need it)
– collapse your hierarchy, merge superclasses with subclasses if they are not different.
– take away the classes that do nothing – move all their features into another classes
– bring down to earth general methods, rename them with current needs, take away unused parameters
Message Chain
Description: You see the next expressions in your code: $object→getA()→getB()→getC().
Problems: This is the case in which a client has to use one object to get another, and then use that one to get to another, etc. Intermediates are tight coupled.
Any change to the intermediate relationships causes the client to make changes.
Decisions:
– try to hide this delegations: create a new method in class A that delegates the call to object B. Now the client does not know about, or depend on, class B.
– remember about LoD (Law of Demeter). Breaking this principle refers to mocking problems in unit-testing.
– perhaps you need to rethink your delegations chain, or it makes sense to extract some methods and put them in the beginning of chain. Moreover, think about chain order, is it the best?
Middle Man
Description: You notice there are classes that do nothing but delegate requests to another classes.
Problems: Probably, you’ve overdone with hiding delegation for resolving Message Chain problem or classes that don’t have any special functionality besides being delegator.
Decisions:
– the simplest way – to delete unnecessary Middle man classes
– make sensible assessment – perhaps Middle man classes reduce tight coupling between business logic classes or they’re part of some structural pattern like Proxy
Envy method
Description: One of the methods is “envious” of another classes’ data – access them more often than your own ones.
Problems: This smell may occur when method was created in the wrong class or used not on purpose. Also the similar situation can take place after class’ fields were moved to Data class
Decisions:
– if all methods exist for serving other class’ data, simply move this method to this class as things that change at the same time should be kept together
– if you see only some part of method use “foreign” data, firstly, extract this part into separate method and only after that move it to another class
– analyze advisability of using separated classes – perhaps your division is redundant and you need to combine them.
