Menu
▾
▴
Re: [Springframework-developer] I don't like the DefaultXmlBeanDefinitionParser
|
From: Steven D. <ste...@gm...> - 2005-02-22 12:48:24
|
I don't think we have heard from Martin why he wants to extend
DefaultXmlBeanDefinitionParser. We don't know if it's in the intrest
of Spring to change something or not.
On Tue, 22 Feb 2005 13:48:46 +0100, Erwin Vervaet
<erw...@er...> wrote:
>
> I think the main reason to use the W3C DOM API directly is to avoid the need
> for an extra dependency (e.g. JDOM) just to parse the XML bean definitions.
> You end up with an "less than elegant" implementation in
> DefaultXmlBeanDefinitionParser, but in this case the benifits outweigh the
> costs.
>
> Erwin Vervaet
> erw...@er...
>
> ----- Original Message -----
> From: Martin Kersten
> To: spr...@li...
>
> Sent: Tuesday, February 22, 2005 1:21 PM
> Subject: [Springframework-developer] I don't like the
> DefaultXmlBeanDefinitionParser
>
>
> Hi folks,
>
> I am currently trying to extend the framework by supporting
> contributions.
> Just to see how it feels.
>
> So I made some investigations in the sourcecode. I don't want to start a war
> about proper design rules, since I am a believer in 'Interface belongs to
> the
> client' stuff and you are appearently not, but this isn't the issue I want
> to
> talk about.
>
> Th implementation I hate most on first sight is the
> XMLBeanDefinitionParser. I know it does what it should but you can read
> this:
>
> /**
> * Make the horrible DOM API slightly more bearable:
> * get the text value we know this element contains.
> */
>
> Well I would agree but it's a bit wired also. You think the DOM API is
> horrible
> and you are still using it? You know what it means to use a
> horrible API? You write a horrible implementation! And thats how it looks.
> It took me more then a gaze to catch the meaning of the parser and I also
> got blown by the code duplication. Since I am in need to extend this class,
>
> So I would like to ask if I may refactor it and commit you a patch (or maybe
> a complete reimplementation)?
>
>
> Cheers,
>
> Martin (Kersten)
>
> PS: By the way, how about 'Hidding 3rd party library behind single
> interface?'
>
> ----- Original Message -----
> From: Martin Kersten
> To: spr...@li...
> Sent: Tuesday, February 22, 2005 12:46 PM
> Subject: Re: [Springframework-developer] Please check these two things
>
>
> Sorry, thought the agreement goes with the callee.
> Ok :-) sorry was a strange day for me, I guess.
>
> Thanks,
>
> Martin (Kersten)
> ----- Original Message -----
>
> From: Juergen Hoeller
> To: spr...@li...
> 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 == 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?
>
>
>
|
