Then I'd probably say that needs to be cleaned up. I was alerted to the
potential issue from IntelliJ's code inspector. Then looking at the code,
it was structured a bit oddly.
I could understand breaking on an exception, but from a cleanup block that
runs at a non-deterministic time, it seems very suspect to me. We can
revert the commit, but it still looks like a place that deserves some amount
of attention.
-- KevinOn 10/15/07 3:23 AM, "Andrus Adamchik" <andru..bjectstyle.org> wrote:
> "break" is not really a noop, as we are dealing with "while(true)" > loop. This may not be coded well, but if I remember correctly the > intent was to get out of the loop for iterated results, without > checking for possible update counts. Sure we should not probably put > it in "finally". > > Andrus > > > On Oct 15, 2007, at 1:27 AM, Kevin Menard wrote: > >> Because it was in a finally block. It appeared as though the break >> was >> added because returning from a finally is a big no-no. But, the >> break was >> essentially a noop for that one case. By inverting the logic and >> dealing >> with only the condition you care about, the other is a de facto noop. >> >> Hopefully I explained that well enough. It's late for me, too. >> I'm in my >> German hotel room whittling away time. >> >> -- >> Kevin >> >> >> On 10/14/07 6:14 PM, "Andrus Adamchik" <andru..bjectstyle.org> wrote: >> >>> Hi Kevin, >>> >>> Nice work cleaning up the code tonight! >>> >>> One question - why do you think the break statement below is not >>> needed? (Keep in mind that it is past 1am in my TZ right now, so I >>> may be asking stupid/obvious stuff :-)) >>> >>> Andrus >>> >>> >>> On Oct 15, 2007, at 12:30 AM, kmenar..pache.org wrote: >>> >>>> Modified: cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/ >>>> src/main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java >>>> URL: http://svn.apache.org/viewvc/cayenne/main/trunk/framework/ >>>> cayenne-jdk1.4-unpublished/src/main/java/org/apache/cayenne/access/ >>>> jdbc/SQLTemplateAction.java?rev=584611&r1=584610&r2=584611&view=diff >>>> ==================================================================== >>>> == >>>> ======== >>>> --- cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/ >>>> main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java >>>> (original) >>>> +++ cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/ >>>> main/java/org/apache/cayenne/access/jdbc/SQLTemplateAction.java Sun >>>> Oct 14 14:30:39 2007 >>>>.. -159,10 +157,7 @@ >>>> t1); >>>> } >>>> finally { >>>> - if (iteratedResult) { >>>> - break; >>>> - } >>>> - else { >>>> + if (!iteratedResult) { >>>> resultSet.close(); >>>> } >>>> } >>> >> >> -- >> >> >> >
--
This archive was generated by hypermail 2.0.0 : Mon Oct 15 2007 - 04:05:45 EDT