Re: svn commit: r584611 - in /cayenne/main/trunk/framework/cayenne-jdk1.4-unpublished/src/main/java/org/ apache/cayenne: ObjectId.java access/jdbc/SQLTemplateAction.java util/IndexPropertyList.java

[Kevin Menard <km...@servprise.com>]

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.

-- 
Kevin


On 10/15/07 3:23 AM, "Andrus Adamchik" <an...@objectstyle.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" <an...@objectstyle.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, kmenard@apache.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();
>>>>                              }
>>>>                          }
>>> 
>> 
>> -- 
>> 
>> 
>> 
> 

--