Method Chaining Gone Wrong

Method chaining is the feature available in many programming languages to stack method calls in one line, with each method accessing the object the previous method returned. Compare:

//Without chaining
MyObject obj = new MyObject();
obj.setVariableA("a");
obj.setVariableB("b");
obj.setVariableThree(3);

//With chaining
MyObject obj = new MyObject()
    .setVariableA("a")
    .setVariableB("b")
    .setVariableThree(3);

The first block’s MyObject class uses traditional setters whereas the second block uses setters that actually return the object being acted upon. This is often used in the Builder Pattern that creates objects with many optional parameters. Here is a brief example:

class Automobile{
	enum Color {
		RED, BLUE, BLACK;
	}
	
	private String make;
	private String model;
	private Color color;
	private double MSRP;
	private int horsepower;
	private int doors;
	private int weight;
	private int cylinders;
	
	public Automobile setMake(String make){
		this.make = make;
		return this;
	}
	
	public Automobile setModel(String model){
		this.model = model;
		return this;
	}
	
	public Automobile setColor(Color color){
		this.color = color;
		return this;
	}
	
	public Automobile setMSRP(double MSRP){
		this.MSRP = MSRP;
		return this;
	}
	
       //Remaining methods omitted for brevity

}

public class CreateCar{
	public static void main(String[] args){
		Automobile myCar = new Automobile()
			.setMake("Alfa Romeo")
			.setModel("8C")
			.setColor(Automobile.Color.BLACK)
			.setHorsepower(444)
			.setMSRP(250000.00d);
	}
}

(Note: There are improvements that should be made to this to truely fit the pattern. See this article for more details.)

However, there is no requirement that method chaining be used with the same object type. Consider something like Integer abc = myArrayList.get(0).get("abc");. Presumably, this code retrieves the map at position 0 in myArrayList and retrieves the Integer mapped to the String “abc.”

This is very tidy and can help to keep code clean, but unfortunately can introduce some problems. First, just looking at the code doesn’t tell me what objects are being called. My IDE will help me out by hovering over each section, but that can be annoying. Second, if something goes wrong on this line like a NullPointerExpection I need to go digging or use the IDE debugger to figure out what is null.

Unfortunately, here’s what happens when a developer goes off the rails:

systemImportDataBean.setType(converterMappingValues.getLocationMap().get(new StringBuffer().append(converterMappingValues.getDescriptionMap().get(systemImportDataBean.getPrimaryId().toLowerCase()).toLowerCase().trim()).append(FieldConstants.OTHER_LOOKUP.toLowerCase()).toString().toLowerCase().trim()));

NO, GOD! NO GOD, PLEASE NO. NO. NO. NO. NOOOOOOO!

Throw us a bone here and at least give us some line breaks! For…clarity(if that’s even possible):

systemImportDataBean.setType(
	converterMappingValues
	.getLocationMap()
	.get(
		new StringBuffer()
		.append(
			converterMappingValues
			.getDescriptionMap()
			.get(
				systemImportDataBean
				.getPrimaryId()
				.toLowerCase()
				)
				.toLowerCase()
				.trim()
			)
			.append(
				FieldConstants.OTHER_LOOKUP
				.toLowerCase()
			)
			.toString()
			.toLowerCase()
			.trim()
		)
	);

GL;HF when something goes wrong with this code. Please be kind to your fellow developers and don’t do something like this. Just because the compiler will let you do this doesn’t mean you should. The compiler isn’t the one will have to dig through this line to fix an error.

Tagged on: , ,

Leave a Reply

Your email address will not be published. Required fields are marked *