Code Smell : Primitive Obsession

Primitive Obsession is when the code relies too much on primitives. What this means is that a primitive value controls the logic in a class and this primitive value is not type safe. For example, there are multiple situations in which we might consider the use of String for comparison and driving the logic even though we are fully aware that it might result in unsafe assignments and invalid equality comparisons. This post discusses an example of primitive obsession and how to get around that using a technique called “Replace Type Code with Class”

Consider the following client code

[sourcecode language=”java”]
public class Client {

public static void main(String[] args) {
Client c = new Client();
c.defaultsToPermissionRequested();
}

public void defaultsToPermissionRequested() {
TwitterPermission permission = new TwitterPermission();
boolean result = permission.getState().equals("REQUESTED");
System.out.println(result);
result = permission.getState().equals("REQEUSTED");
System.out.println(result);
}
}
[/sourcecode]

As you can see, the above code is not type safe. It is prone to errors like spelling mistakes both for the client checking the state as well as for the code which is setting the state.
Consider this code segment
[sourcecode language=”java”]
assertEquals("REQUESTD", permission.state());
[/sourcecode]

here there is a spelling mistake and that is enough for the code to go for a toss. Interesting thing here is that the compiler is not going to complain and you would know about the error only as a logical error.

Now, consider the following class

[sourcecode language=”java”]
public class TwitterPermission {
private String state;
private boolean granted;
public final static String REQUESTED = "REQUESTED";
public final static String CLAIMED = "CLAIMED";

public TwitterPermission() {
state = REQUESTED;
granted = false;
}

public void claimed() {
if (state.equals(REQUESTED))
state = CLAIMED;
}

public String getState() {
return state;
}
}
[/sourcecode]

The type-unsafe field in TwitterPermission is called state. It is assigned to and compared against a family of String constants also defined within TwitterPermission. The way to make this type safe is to make the status a class rather than a String. One of the ways to do that in Java is through the use of enums. Another similar way to rather create a class.
So we would have something like a PermissionState which represents the state for the TwitterPermission

[sourcecode language=”java”]
public class PermissionState {

private String name;

public PermissionState(String name) {
this.name = name;
}
public final static PermissionState REQUESTED = new PermissionState("REQUESTED");
public final static PermissionState CLAIMED = new PermissionState("CLAIMED");

@Override
public String toString() {
return name;
}
}
[/sourcecode]

Here, we are doing the following

1) The state is no longer defined in TwitterPermission
2) It is a separate class which would be used by TwitterPermission for comparison and logic, however, this time it would not be a String

Let us see what does the refactored TwitterPermission look like

[sourcecode language=”java”]
public class TwitterPermissionRefactored {
private boolean granted;

private PermissionState permission;

public TwitterPermissionRefactored() {
setState(permission.REQUESTED);
granted = false;
}

public void claimed() {
if (getState().equals(permission.REQUESTED))
setState(PermissionState.CLAIMED);

}

public String getState() {
return permission.toString();
}

private void setState(PermissionState permission) {
this.permission = permission;
}

}
[/sourcecode]

and the refactored client would look like

[sourcecode language=”java”]
public class ClientRefactored {

public static void main(String[] args) {
ClientRefactored c = new ClientRefactored();
c.defaultsToPermissionRequested();
}

public void defaultsToPermissionRequested() {
TwitterPermissionRefactored permission = new TwitterPermissionRefactored();

boolean result = PermissionState.REQUESTED.toString().equals(permission.getState());
System.out.println(result);
}

}
[/sourcecode]

Now TwitterPermission’s assignments to its permission field and all equality comparisons with its permission field are type-safe.

Written by 

Vikas is the CEO and Co-Founder of Knoldus Inc. Knoldus does niche Reactive and Big Data product development on Scala, Spark, and Functional Java. Knoldus has a strong focus on software craftsmanship which ensures high-quality software development. It partners with the best in the industry like Lightbend (Scala Ecosystem), Databricks (Spark Ecosystem), Confluent (Kafka) and Datastax (Cassandra). Vikas has been working in the cutting edge tech industry for 20+ years. He was an ardent fan of Java with multiple high load enterprise systems to boast of till he met Scala. His current passions include utilizing the power of Scala, Akka and Play to make Reactive and Big Data systems for niche startups and enterprises who would like to change the way software is developed. To know more, send a mail to hello@knoldus.com or visit www.knoldus.com

2 thoughts on “Code Smell : Primitive Obsession

  1. Your code has public constructor and miss equals/hashcode. This force you to use toString to do the actual comparison. Because it is non standard, it is likely to confuse your team mates and will produce bugs.

    Really you should use java enum instead :

    public enum PermissionState { REQUESTED, CLAIMED}

    and then compare directly :

    boolean result = PermissionState.REQUESTED.equals(permission.getState());

    1. Nicolas, what you are saying is correct. If you notice in the post, I mention it too that in Java you should use enums. The idea here is not to portray the Java case (though it is used as a medium of expression). The idea is to show that primitive obsession should and can be avoided by using classes, it might be the way represented above or enums if the language in which you want to avoid primitives based logic supports it. I hope this makes the intent clear.

Leave a Reply

%d bloggers like this: