Thoughts about code readability

May 15, 2005

There is one thought that crossed my mind: Since braces do nothing, they are basically a waste. Python recognized this and uses indentation to create blocks.

The following is a very common method in Java programs:

public void setName(String name)
{
    this.name = name;
}

This method isn’t very significant for the program and it’s very unlikely to contain a defect. My editor window shows about 40 lines - after that I have to scroll down. Now a method that is insignificant and is unlikely to have a defect takes up about 10 % of the available space. Even worse I might have 9 other instance variables that need encapsulation desperatly, so I end up having a page of setter methods. If I want to see what the class really does, I have to scroll.

From that point of view, the following is better, but not much.

public void setName(String name) {
    this.name = name;
}

One line saved per setter or getter. But it really doesn’t solve the problem. Why waste even three lines of code for doing almost nothing?

There is another problem with those getters and setters: clutter. If one of the setter methods does something more than just setting the instance variable it might be lost on first sight in the noise the others provide. Here is a class with a few attributes that are encapsulated by getter and setter methods. Answer as fast as possible: Which attribute(s) can’t be set and which method isn’t your normal “getter” method?

public class Test
{
	private String firstname;
	private String lastname;
	private String address;
	private String city;
	private String country;
	private String title;
	private String size;
	private String haircolor;

	public String getAddress()
	{
		return address;
	}

	public String getCity()
	{
		return city;
	}

	public String getCountry()
	{
		return country;
	}

	public String getFirstname()
	{
		return firstname;
	}

	public String getHaircolor()
	{
		return haircolor;
	}

	public String getLastname()
	{
		return lastname;
	}

	public String getName()
	{
		return getFirstname() + getLastname();
	}

	public String getSize()
	{
		return size;
	}

	public String getTitle()
	{
		return title;
	}

	public void setAddress(String string)
	{
		address = string;
	}

	public void setCity(String string)
	{
		city = string;
	}

	public void setFirstname(String string)
	{
		firstname = string;
	}

	public void setHaircolor(String string)
	{
		haircolor = string;
	}

	public void setLastname(String string)
	{
		lastname = string;
	}

	public void setSize(String string)
	{
		size = string;
	}

	public void setTitle(String string)
	{
		title = string;
	}

}

OK. Most getters and all setters were generated by Eclipse - you might be able to do better manually. The class already contains 100 lines of code that do pretty much nothing. Things get even worse when you write a documentation comment for each method (especially if it says something meaningless like “sets the size”)!

Same class, same questions, different style:

public class Test
{
	private String firstname;
	private String lastname;
	private String address;
	private String city;
	private String country;
	private String title;
	private String size;
	private String haircolor;

	public String getAddress() { return address; }
	public void setAddress(String string) {	address = string; }

	public String getCity() { return city; }
	public void setCity(String string) { city = string; }

	public String getFirstname() { return firstname; }
	public void setFirstname(String string)	{ firstname = string; }

	public String getHaircolor() { return haircolor; }
	public void setHaircolor(String string) { haircolor = string; }

	public String getLastname() { return lastname; }
	public void setLastname(String string) { lastname = string; }

	public String getTitle() { return title; }
	public void setTitle(String string) { title = string; }

	public String getSize()	{ return size; }
	public void setSize(String string) { size = string; }

	public String getCountry() { return country; }

	public String getName()
	{
		return getFirstname() + getLastname();
	}

}

The class shrinks to about 50 lines of code, does the same thing and above all, points out the important parts of this class. It almost fits on one screen, too. A bit unfamiliar maybe - but nothing you wouldn’t be able to get used to.

Of course, if you dare to ignore general wisdom of encapsulation you can make it even clearer:

public class Test
{
	public String firstname;
	public String lastname;
	public String address;
	public String city;
	public String title;
	public String size;
	public String haircolor;

	private String country;

	public String getName()
	{
		return firstname + lastname;
	}

	public String getCountry()
	{
		return country;
	}
}

I know this is considered absolutly evil and has some drawbacks. But if you look at it in isolation, you can clearly see the beauty of it. You have the same information on 20 lines (= half a screen).

If you just considered ease of reading, which version would you choose?

Posted by Jerome Mueller