A wise old hermit known only as Ara Abrahamian <[EMAIL PROTECTED]> once
said:
> > Maybe to avoid a new Tag and do not have to remember to add it, should
> > I add a parameter to have something like :
> >
> > <XDtMethod:ifHasMethod name="..." setCurrentMethod="true/false">
> >
> > With a default to "true" meaning we set the current method by default.
>
> Disagree.
Personally, I don't see anything wrong with needing a new tag. On the
other hand, before we switch context to another method we're probably
always going to check it exists, so I guess it could be done like this. I
think a separate tag with a more descriptive name would be clearer,
though.
> > > Wasn't that the point of the hasMethod tag we discussed
> > > recently (thread
> > > with subject "entitycmp.j") which Ara removed because "the
> > > code was the
> > > same"? I assumed that ifHasMethod was therefore already
> > > doing this, since
> > > that was the point of the removed code...
>
> Well, I looked at the removed code again. They are the same, except one
> sets current method the other one doesn't. But the point is why should
> it set current method after all?
Because it had to for the ifIsNotAbstract to be checking the correct
method. And it's only sensible for that tag to check the method exists
before it switches to it; if that made it look too similar to ifHasMethod,
then perhaps we should have had a private methodExists(), which they both
called. It needed a better name though - hasMethod was too much like
ifHasMethod and sounded like another similar test.
forMethod/withMethod/inMethod would be better.
> No need really, specially for a tag
> which should just test something. Here is the old code:
>
> <XDtMethod:ifHasMethod name="<XDtMethod:setterMethod/>" ...>
> public void <XDtMethod:setterMethod/>( ... ) <XDtMethod:exceptionList
> method="<XDtMethod:setterMethod/>"/>
> {
> <XDtMethod:hasMethod name="<XDtMethod:setterMethod/>"
> parameters="<XDtMethod:methodType/>">
> <XDtMethod:ifIsNotAbstract>
> super.<XDtMethod:setterMethod/>(<XDtMethod:propertyName/>);
> </XDtMethod:ifIsNotAbstract>
> </XDtMethod:hasMethod>
>
> ifHasMethod doesn't set it, so we need to specify the method we're
> looking for in XDtMethod:exceptionList/etc too.
But if we could change the context to the other method, we wouldn't need
to be specifying this same method name on all these other tags (which
would improve performance as well, since we also avoid all the nested
calls to get the method name within the name="..." parameters).
> hasMethod does set
> current method so the nested ifIsNotAbstract (which uses
> getCurrentmethod().isAbstract()) uses it. But why should it set it?
> Current getter method is either abstract or not.
...
> > > > The issue I have is that XDtMethod:ifIsNotAbstract does not work
> > > > in case a setter method is not abstract while a getter is.
>
> Yup. Doesn't work for this case.
Which is why it needs to change the current method; we aren't interested
if the current method (generally the getter within the
forAllPersistentFields loop, though it may not always be) is abstract, but
in the one we've just checked the existence of.
> > > > Another solution could be to parametrized
> > > XDtMethod:ifIsNotAbstract,
> > > > but I think setting the current method will makes things clearer.
>
> Yup, parameterize it, like XDtMethod:exceptionList above:
> <XDtMethod:ifIsNotAbstract method="<XDtMethod:setterMethod/>"/>
Don't forget the parameters - there might be other setFoobar() methods
with different arguments, and we want to check the right one...
If we don't have a tag to change the current method, then potentially
*every* similar tag (all those in the method namespace?) is going to need
method="..." and parameters="..." arguments - they may not be necessary
for the standard templates, but we've got to cater for people's custom
templates too.
Also, any time there's a whole bunch of these tags used together in a
template, it would make things much simpler to switch to the other method
to do them all rather than having the same method and parameters arguments
specified on every single tag.
> You know, ifHasMethod should just check it, nothing more.
One tag, one action. Sounds good to me.
> Btw, isn't it
> needed somewhere to check a particular method's abstractness? I mean not
> the current method.
If it is, then I guess it won't always work properly at the moment :-)
> So the 'method' optional parameter is something good.
Agreed. But an additional tag to change the current method could also be
good, where it makes the templates simpler.
One other thing that occurs to me - we have an ifHasMethod tag, but do we
have an equivalent one for testing if a given method *doesn't* exist (e.g.
so we can add one in the subclass)?
Andrew.
_______________________________________________
Xdoclet-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/xdoclet-devel
