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

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

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?