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

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);
	}
}

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

assertEquals("REQUESTD", permission.state()); 

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

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;
	}
}

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

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;
	}
}

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

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;
		}

}

and the refactored client would look like

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);
	}

}

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

About Vikas Hazrati

Vikas is the Founding Partner @ Knoldus which is a group of software industry veterans who have joined hands to add value to the art of software development. 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). To know more, send a mail to hello@knoldus.com or visit www.knoldus.com
This entry was posted in Agile, Architecture, Java and tagged , , , . Bookmark the permalink.

2 Responses to Code Smell : Primitive Obsession

  1. Nicolas says:

    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());

    • 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

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s