Actually, I suspect that the correct solution is to allow the
ClassCastException to propagate. I don't see how having random,
non-Integer objects map to row-not-present is ever what the developer
intended. This code looks like it was added to avoid exceptions during
rendering, but I feel it is more important to find these problems during
development.
-- Blake Sullivan
Simon Lessard said the following On 10/16/2008 12:50 PM PT:
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
