Skip to main content

JTable demo is suboptimal code

3 replies [Last post]
doubletrouble
Offline
Joined: 2007-11-15
Points: 0

The JTable demo
http://java.sun.com/docs/books/tutorial/uiswing/components/table.html
has the following bit of code:

<br />
	static class DateRenderer extends DefaultTableCellRenderer {<br />
	   DateFormat formatter;<br />
	   public DateRenderer() { super(); }</p>
<p>	   public void setValue(Object value) {<br />
		   if (formatter==null) {<br />
			   formatter = DateFormat.getDateInstance();<br />
		   }<br />
		   setText((value == null) ? "" : formatter.format(value));<br />
	   }<br />
	}<br />

near the start of this section:
http://java.sun.com/docs/books/tutorial/uiswing/components/table.html#re...

I think that this code is suboptimal: it will cause the creation of a dedicated DateFormat instance for EVERY cell that uses a DateRenderer. That is a waste of cpu and memory.

Plus the lazy initialization is simply silly. (Please, people, use lazy initialization only for what it is really useful for: avoiding expensive allocations of objects that may never be used. Do NOT use it for allocations that have to take place anyways for your object to be used--all you are doing in this case is adding code bloat and lowering performance.)

I think that the optimal code is to make formatter a static field, so that a single instance can be shared across all instances. This should be perfectly safe, because Swing is single threaded (in particular, all cell rendering calls should be made on the Event Dispatch Thread). Here is the proposed new code:

<br />
	static class DateRenderer extends DefaultTableCellRenderer {<br />
		private static final DateFormat formatter = DateFormat.getDateInstance();</p>
<p>		public DateRenderer() { super(); }</p>
<p>		public void setValue(Object value) {<br />
			setText((value == null) ? "" : formatter.format(value));<br />
		}<br />
	}<br />

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
amfwaddi
Offline
Joined: 2005-11-10
Points: 0

I think you need to show that the same DateCellRenderer isn't used for the whole table.

Andrew

doubletrouble
Offline
Joined: 2007-11-15
Points: 0

Andrew: point noted.

I wrote that over 3.5 years ago. Totally forgot about it. Looking back now, I cannot see why I thought that a new instance is created for every cell.

I tried just now to find some example code on that page which actually uses DateRenderer, but I do not see any.

So maybe you are right. That would certainly be more efficient.

I stand by my comment about lazy initialization being bad in this case (and horribly over used by many programmers in other contexts).

teleporter
Offline
Joined: 2012-05-14
Points: 0

Nicely done! :)