Code Smells

naturally, the credit for the contents here go to https://refactoring.guru/refactoring/catalog

bloaters#

these smells make the code hard to work with. they do not usually appear immediately, but are a consequence of program evolution and accumulation over time:

long method#

when the method in a class is too long. you can split a method up into many smaller methods.

large class#

a consequence of the fact that it is easier to add functionality to an existing class (as a programmer), than to create a new class.

primitive obsession#

these get spread wide and far. it emerges from a degree of laziness. passing around data as:

String email;
int age;
double latitude;
double longitude;

whilst it should really look like:

EmailAddress email;
Age age;
GeoPoint location;

we distinguish between creating a meaningful class and a “data class” (which is another smell); data class :: contains only fields (and maybe getters / setters), but no meaningful behaviour value object :: contains invariants, validation, domain behaviour, meaninful operations, encapsulation.

in summary, the value object knows it exists and exists for a reason – to model a concept. unlike the data class, which just moves the smell and exists to “carry data”

data clumps#

a sibling smell to primitive obsession, but with the added concern that the relationship between values is not explicit.

in the case of primitive obsession, a single concept was being represented using raw primitives as opposed to a domain type.

here, we have a set of variables repeatedly appearing together that should be treated as one concept:

double x, y;
double latitude, longitude;
int day, month, year;

but often times, the data clumps are indeed made up of primitives, so the smells reinforce each other.

long parameter list#

when the parameters are all related:

void createOrder(
    String customerId,
    String email,
    String street,
    String city,
    String postcode,
    String country,
    BigDecimal amount,
    Currency currency
)

we can create a value object as with the primitive obsession fix, to call the method as such:

void createOrder(CustomerId customerId, Address address, Money total)

object-orientation abusers#

alternative classes with different interfaces#

when two classes perform identical functions but have different method names.

happens when the programmer doesn’t know about existing functionality.

refused bequest#

when subclasses inherit functionality that they do not need

switch statements#

multiple switch statements in multiple places are the smell.

usually one switch is okay, i.e. to create objects in the factory method or otherwise.

temporary field#

temporary fields get their value (and thus are needed by objects) only under certain circumstances.

outside of these circumstances, they’re empty.

change preventers#

divergent change#

when the same class is changed for many reasons.

violation of the single responsibility principle

parallel inheritance hierarchies#

whenever you create a subclass for a class, you find yourself needing to create a subclass for another class

shotgun surgery#

when a change to one class necessitates many small changes to other classes.

dispensables#

comments#

usually an indication of poorly named variables, bad logic or otherwise a band-aid for bad code.

duplicate code#

when two code fragments look almost identical

data class#

classes that do not provide any additional functionality. they only contain fields with at most methods for getting and setting these variables.

additionally, these classes cannot independently operate on the data that they own.

dead code#

when a variable, parameter, field, method or class is no longer used – usually because it is obsolete

lazy class#

understanding and maintaining classes always costs time and money. so if a class doesn’t do enough to earn your attention, it should be deleted.

speculative generality#

when we write code for “future use”, but that code is speculative and not connected to the “in use” code in any way.

couplers#

feature envy#

when a method accesses the data of another object more than its own data

incomplete library class#

eventually a library will stop meeting our needs and we’ve now got heaps of work-arounds and wrappers.

this causes our code to become less maintainable and increases technical debt.

as such, this library is now insufficient for our particular requirements.

middle man#

if a class performs only one action; delegating work to another class - why keep it at all?

inappropriate intimacy#

one class uses the internal fields and methods of another class

message chains#

when you see a series of calls resembling a->b()->c()->d().

violation of the law of demeter