[xwiki-devs] Question about AbstractMacro API
Hi all, While working on macro categories implementation I came across a small problem that made me little uncomfortable with AbstractMacro API. I'm writing this email just to make sure that I'm not making things worse in the public API. Currently AbstractMacro has 4 constructors: * public AbstractMacro(String description); * public AbstractMacro(String description, ContentDescriptor contentDescriptor) * public AbstractMacro(String description, Class< ? > parametersBeanClass) * public AbstractMacro(String description, ContentDescriptor contentDescriptor, Class< ? > parametersBeanClass) Now, I wanted to introduce a MacroDescriptor::getDefaultCategory() method and I also wanted to make this new parameter available in AbstractMacro constructors (makes sense). Now the problem with this is, If I introduce an optional 'defaultCategory' parameter into AbstractMacro, this will make the total number of constructors 8. What will happen if someone wants to introduce a MacroDescriptor::getIcon() method some time later? One solution is to have setXXXX methods for those parameters instead. But then a slight problem occurs where AbstractMacro needs to somehow call setXXX methods on it's internal MacroDescriptor instance. It's not that we can't cook comething up, but it's going to look hackish. My question is, why do we have to make AbstractMacro constructors so simple? Can't we have these two constructors only? * public AbstractMacro(); * public AbstractMacro(MacroDescriptor); The problem with this approach is that a client will have to do little more work like; ContentDescriptor myContentDescriptor = new DefaultContentDescriptor("This is my content", true); MacroDescriptor myMacroDescriptor = new DefaultMacroDescriptor("This is my macro", myContentDescriptor, beanManager.getBeanDescriptor(MyParametersClass.Class)) super(myMacroDescriptor); Even though this is too much of work, I think it will make the API more extensible an clean. This is only my opinion. I didn't want to drag this issue this far but I just couldn't feel myself comfortable with the way I'm going to implement the macro categories support. Up to you. Thanks. - Asiri
ContentDescriptor myContentDescriptor = new DefaultContentDescriptor("This is my content", true);
MacroDescriptor myMacroDescriptor = new DefaultMacroDescriptor("This is my macro", myContentDescriptor, beanManager.getBeanDescriptor(MyParametersClass.Class))
super(myMacroDescriptor);
Correction: This is wrong. It should be setDescriptor(myMacroDescriptor) Thanks. - Asiri
On Thu, Aug 6, 2009 at 12:50, Asiri Rathnayake<[email protected]> wrote:
Hi all,
While working on macro categories implementation I came across a small problem that made me little uncomfortable with AbstractMacro API. I'm writing this email just to make sure that I'm not making things worse in the public API.
Currently AbstractMacro has 4 constructors:
* public AbstractMacro(String description);
* public AbstractMacro(String description, ContentDescriptor contentDescriptor)
* public AbstractMacro(String description, Class< ? > parametersBeanClass)
* public AbstractMacro(String description, ContentDescriptor contentDescriptor, Class< ? > parametersBeanClass)
Now, I wanted to introduce a MacroDescriptor::getDefaultCategory() method and I also wanted to make this new parameter available in AbstractMacro constructors (makes sense). Now the problem with this is, If I introduce an optional 'defaultCategory' parameter into AbstractMacro, this will make the total number of constructors 8. What will happen if someone wants to introduce a MacroDescriptor::getIcon() method some time later?
One solution is to have setXXXX methods for those parameters instead. But then a slight problem occurs where AbstractMacro needs to somehow call setXXX methods on it's internal MacroDescriptor instance. It's not that we can't cook comething up, but it's going to look hackish.
My question is, why do we have to make AbstractMacro constructors so simple? Can't we have these two constructors only?
* public AbstractMacro();
* public AbstractMacro(MacroDescriptor);
The problem with this approach is that a client will have to do little more work like;
ContentDescriptor myContentDescriptor = new DefaultContentDescriptor("This is my content", true);
MacroDescriptor myMacroDescriptor = new DefaultMacroDescriptor("This is my macro", myContentDescriptor, beanManager.getBeanDescriptor(MyParametersClass.Class))
super(myMacroDescriptor);
Even though this is too much of work, I think it will make the API more extensible an clean.
This is not possible in java, the super()/this() has to be the first thing call in the constructor.
This is only my opinion. I didn't want to drag this issue this far but I just couldn't feel myself comfortable with the way I'm going to implement the macro categories support.
Up to you.
Thanks.
- Asiri _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
On Thu, Aug 6, 2009 at 12:50, Asiri Rathnayake<[email protected]> wrote:
Hi all,
While working on macro categories implementation I came across a small problem that made me little uncomfortable with AbstractMacro API. I'm writing this email just to make sure that I'm not making things worse in the public API.
Currently AbstractMacro has 4 constructors:
* public AbstractMacro(String description);
* public AbstractMacro(String description, ContentDescriptor contentDescriptor)
* public AbstractMacro(String description, Class< ? > parametersBeanClass)
* public AbstractMacro(String description, ContentDescriptor contentDescriptor, Class< ? > parametersBeanClass)
Now, I wanted to introduce a MacroDescriptor::getDefaultCategory() method and I also wanted to make this new parameter available in AbstractMacro constructors (makes sense). Now the problem with this is, If I introduce an optional 'defaultCategory' parameter into AbstractMacro, this will make the total number of constructors 8. What will happen if someone wants to introduce a MacroDescriptor::getIcon() method some time later?
One solution is to have setXXXX methods for those parameters instead. But then a slight problem occurs where AbstractMacro needs to somehow call setXXX methods on it's internal MacroDescriptor instance. It's not that we can't cook comething up, but it's going to look hackish.
My question is, why do we have to make AbstractMacro constructors so simple?
Because we want to make as easy as possible to write and contribute macros :)
Can't we have these two constructors only?
* public AbstractMacro();
* public AbstractMacro(MacroDescriptor);
The problem with this approach is that a client will have to do little more work like;
ContentDescriptor myContentDescriptor = new DefaultContentDescriptor("This is my content", true);
MacroDescriptor myMacroDescriptor = new DefaultMacroDescriptor("This is my macro", myContentDescriptor, beanManager.getBeanDescriptor(MyParametersClass.Class))
super(myMacroDescriptor);
Even though this is too much of work, I think it will make the API more extensible an clean.
This is only my opinion. I didn't want to drag this issue this far but I just couldn't feel myself comfortable with the way I'm going to implement the macro categories support.
Up to you.
Thanks.
- Asiri _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne
Hi,
My question is, why do we have to make AbstractMacro constructors so simple?
Because we want to make as easy as possible to write and contribute macros :)
Yes, I agree with you. But I wondered, if a user is writing a java macro, he knows java! But again a constructor like * public AbstractMacro(MacroDescriptor); Would make his life bit worse and doing something like: super(new DefaultMacroDescriptor(... new DefaultContentDescriptor())) is ugly. The other problem is He has to know about BeanManager and component injection with this sort of a constructor.. So yes, it makes his life hard. Still, I'm a bit uncomfortable with the fact that introducing a new attribute to MacroDescriptor and being unable to expose it to the macro author in a simple way. Anyway, I'm halfway down the implementation so, it's better if things do not change now ;) - Asiri
Can't we have these two constructors only?
* public AbstractMacro();
* public AbstractMacro(MacroDescriptor);
The problem with this approach is that a client will have to do little more work like;
ContentDescriptor myContentDescriptor = new DefaultContentDescriptor("This is my content", true);
MacroDescriptor myMacroDescriptor = new DefaultMacroDescriptor("This is my macro", myContentDescriptor, beanManager.getBeanDescriptor(MyParametersClass.Class))
super(myMacroDescriptor);
Even though this is too much of work, I think it will make the API more extensible an clean.
This is only my opinion. I didn't want to drag this issue this far but I just couldn't feel myself comfortable with the way I'm going to implement the macro categories support.
Up to you.
Thanks.
- Asiri _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
-- Thomas Mortagne _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
participants (2)
-
Asiri Rathnayake -
Thomas Mortagne