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

[Martin K. <Mar...@St...>]

Sorry, thought the agreement goes with the callee.=20
Ok :-) sorry was a strange day for me, I guess.

Thanks,

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


  Actually, I have *not* replaced this with a =3D=3D 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 =3D=3D =
makes=20
    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=20
    start with if(this=3D=3Dobject) return true;. How huge is the =
estimated
    performance gain?


    Cheers,

    Martin (Kersten)
      ----- Original Message -----=20
      From: Juergen Hoeller=20
      To: spr...@li...=20
      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 =3D=3D 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.=20
        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 =3D=3D =
this.parameterQueue.getLast()

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