I hate API's like this, especially in private methods. If the thing
expects an Interger, why not just make the rowKey an integer? Grrrr.
Keeping things generic objects sucks.
Now off my soapbox. :)
+1 Simon, doing the instanceof is on orders of magnitude less expensive
then throwing an exception and assembling a stacktrace. It appears from
this code that a non integer is an acceptable (albeit unexpected)
codepath and we should never use exceptions for flow control. Good catch!
Scott
Simon Lessard wrote:
Hi all,
I'm working on an ADF Faces Rich Client atm and as some may know, that
library is based on Trinidad. Anyway, while debugging for a completely
different issue, I found out the following in SortableModel class:
private int _toRowIndex(Object rowKey)
{
if (rowKey == null)
return -1;
try
{
return ((Integer)rowKey).intValue();
}
catch (ClassCastException e)
{
_LOG.warning("INVALID_ROWKEY", new Object[]{rowKey ,
rowKey.getClass()});
_LOG.warning(e);
return -1;
}
}
I think we should change that to something like the following to not
shallow an Exception that's more expensive to create than an
instanceof check. Note that for some reason I get one such exception
per request currently so this is kind of annoying ;) :
private int _toRowIndex(Object rowKey)
{
if (rowKey == null)
{
return -1;
}
else if (rowKey instanceof Integer)
{
return ((Integer)rowKey).intValue();
}
else
{
_LOG.warning("INVALID_ROWKEY", new Object[]{rowKey ,
rowKey.getClass()});
return -1;
}
}
What do you think?
~ Simon
