Re: [Springframework-developer] Please check these two things

[Juergen H. <ju...@in...>]

Actually, I have *not* replaced this with a == comparison of the arrays:
Instead, BatchSqlUpdate is storing clones of the passed-in arrays now, for
execution on flush. This avoids any side effects in the first place (even if
the passed-in arrays are changed afterwards or reused for multiple update
inovcations), and the overhead of cloning an array should be acceptable
(after all, we're talking about database update operations here).

Juergen


  -----Original Message-----
  From: spr...@li...
[mailto:spr...@li...]On Behalf Of
Martin Kersten
  Sent: Tuesday, February 22, 2005 12:04 PM
  To: spr...@li...
  Subject: Re: [Springframework-developer] Please check these two things


  But isn't this bogus thinking? I mean replacing .equals with == makes
  the implementation more strickt and reduces semantical informations.
  We are thinking about objects and there is no performance gap
  to justify this modification.

  I wouldn't do it. I just would ensure that equals implementations
  start with if(this==object) return true;. How huge is the estimated
  performance gain?


  Cheers,

  Martin (Kersten)
    ----- Original Message -----
    From: Juergen Hoeller
    To: spr...@li...
    Sent: Tuesday, February 22, 2005 10:06 AM
    Subject: Re: [Springframework-developer] Please check these two things


    Well-spotted!

    ConcurrencyThrottleInterceptor should indeed use an internal monitor to
avoid any potential for side effects. I doubt that this has caused any issue
in practice, but it's nevertheless cleaner.

    That check in BatchSqlUpdate is not supposed to compare the elements but
just the array reference: Repeated update invocations should not pass-in the
same array instance repeatedly, with modified elements. Of course, a ==
check would be sufficient for this. I've reworked that part a bit
differently, though: BatchSqlUpdate stores a clone of the passed-in array
now, so there shouldn't be a need for such a check anymore.

    Juergen


      -----Original Message-----
      From: spr...@li...
[mailto:spr...@li...]On Behalf Of
Dave Brosius
      Sent: Tuesday, February 22, 2005 8:08 AM
      To: spr...@li...
      Subject: [Springframework-developer] Please check these two things


      These may be problems, and then again maybe not. But they seem
odd/wrong to me


      1) In
org.springframework.aop.interceptor.ConcurrencyThrottleInterceptor

      in method invoke

      uses wait on 'this'

      In my mind you are exposing your synchronization strategies as a
public artifact, which leaves this class open to failure due to client code.
      The client code may unwittingly us an instance of this class to do
it's own synchronization, and totally screw up this class.
      I would recommend doing synchronizations (especially the use of
wait/notify) on a private member so client code can not effect it.



      2) In org.springframework.jdbc.object.BatchSqlUpdate

      in method update, you do

        if (!this.parameterQueue.isEmpty() &&
args.equals(this.parameterQueue.getLast())) {

      this is the same as using     args == this.parameterQueue.getLast()

      or in other words, are these objects the same object. I assume you
want to compare the elements of the array?