On Mon, 2 Jan 2006, Thomas Vandahl wrote:
Thomas Fischer wrote:
Just out of curiosity: Why is it better to iterate over the entries of a
map instead of the keys, if one needs both key and entry ? I know it is
better to use the Entry set if one does not need the key, but here the key
is needed...
In the given loop both parts of the entry are used, see the log.debug() call.
The code before took the key from the iterator and used get(key) to read the
value. This is basically a search call which is rather expensive. If you need
both, key and value, the entryset-iterator provides them without an
additional search operation.
That sounds reasonable. I did not know that, and it certainly makes sense
in most places. However, I just happened to run the runtime test an hour
ago on jdk 1.4.2_05, and it failed in the test case testSerialisation in
line 568. The problem is that Criteria inherits from a Hashtable, and it
overrides the get() method from the Hashtable. So criteria.get() does
something else than entry.getValue(). In the case of Criteria, I'm
afraid we have to switch back to the old code which iterates over the
keys.
However, this was not clear to me before I debugged into it. In my
opinion, the Criteria class being a child of Hashtable is very bad
style, because it uses undocumented features of the Hashtable class. I'd
like to change that, and a few other implementation issues (like
Criterion not being a static inner class) in some stage.
[...]
So in my code, I usually close stuff in the try block, set it to null, and
do a not null check in the finally block. If the reference to the resource
is not null, then I know an error has occured, try to close the other stuff
nevertheless, and ignore all exceptions which occur during clean-up (as
there is already an exception in the pipeline). This has the desired
behaviour for both cases. I would suggest that we do the same here. I can
do it, if noone objects.
If everyone agrees, we should also look at the other places where stuff
should be closed again (the connection in doSelect(criteria) and the like).
Feel free. I guess there still are a couple of places where the same things
are handled in different ways. To improve code quality, it would be desirable
to clean this up, even if it is not strictly an error.
I will put it on my todo list.
I would not use throwTorqueException() on a Torque exception, but simply
rethrow it.
I thought this was the exact purpose of throwTorqueException(). See the code
of this method.
Shame on me for my comment. I should have looked at the code.
Thomas
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
