<div dir="ltr">This refactoring is quite important, we should talk about this during the next meeting. I would be more than happy to try to do it, but I fear to break any tests or backward compatibility issue...</div><div class="gmail_extra"><br><div class="gmail_quote">2014-10-27 13:43 GMT+01:00 George Gastaldi <span dir="ltr"><<a href="mailto:ggastald@redhat.com" target="_blank">ggastald@redhat.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
Great, so +1 to that.<div><div class="h5"><br>
<br>
<div>On 10/27/2014 10:41 AM, Antonio
Goncalves wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">The extra layer of AbstractValidationCommand,
AbstractCDICommand, AbstractJPACommand (notice that this layer
already exists for JSF) is justified by
overriding isProjectRequired and getPrerequisiteCommands (all
the Java EE commands need a project and need to be setup, see
the code below).
<div><br>
</div>
<div>Then, if you say that the interface is optional, I would
get rid of it.</div>
<div><br>
</div>
<div>All in all, I think that homogenize the code is very
important for new comers (like me). Creating a new command is,
mostly, copy/paste + adding some specific logic. And depending
which class you copy/paste, you end up with very different
code.</div>
<div><br>
</div>
<div>Antonio<br>
<div><br>
</div>
<div><br>
</div>
<div>
<p><span>@Override<br>
</span><span><b>protected boolean </b></span>isProjectRequired()<br>
{<br>
<span><b>return true</b></span>;<br>
}<br>
</p>
</div>
<div> @Override</div>
<div> public NavigationResult
getPrerequisiteCommands(UIContext context)</div>
<div> {</div>
<div> NavigationResultBuilder builder =
NavigationResultBuilder.create();</div>
<div> Project project = getSelectedProject(context);</div>
<div> if (project != null)</div>
<div> {</div>
<div> if (!project.hasFacet(CDIFacet.class))</div>
<div> {</div>
<div> builder.add(CDISetupCommand.class);</div>
<div> }</div>
<div> }</div>
<div> return builder.build();</div>
<div> }</div>
<div><br>
</div>
<div class="gmail_extra"><br>
<div class="gmail_quote">2014-10-27 13:35 GMT+01:00 George
Gastaldi <span dir="ltr"><<a href="mailto:ggastald@redhat.com" target="_blank">ggastald@redhat.com</a>></span>:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> Hi Antonio,<br>
<br>
Yeah, I think that's fine. The idea of having an
interface is to reference the next command in the
next() method (or as a prerequisite), but that is
optional.<br>
I think that would be a good idea, if these
specializations had enough code to justify their
existence.
<div>
<div><br>
<br>
<br>
<div>On 10/27/2014 02:58 AM, Antonio Goncalves
wrote:<br>
</div>
</div>
</div>
<blockquote type="cite">
<div>
<div>
<div dir="ltr">
<div class="gmail_extra">
<div>Hi all,</div>
<div><br>
</div>
<div>I'm trying to add more commands in
Forge... but I have to say, I'm a bit
lost. So, I've made a quick UML class
diagram. </div>
<div><br>
</div>
<div>As you can see in the attached diagram
(UIForge.png), most of the Java EE
commands extend AbstractJavaEECommand,
which makes sense. But not all of them
(NewBeanCommand (CDI),
ValidationNewAnnotationCommandImpl,
NewQualifierCommand....). And some times
you have an extra level of abstraction
(AbstractFacesCommand). Same for the Java
commands. JavaClassCommandImpl extend
AbstractJavaSourceCommand but
JavaAddAnnotationCommand and
JavaFieldCommand inherit from
AbstractProjectCommand.</div>
<div><br>
</div>
<div><br>
</div>
<div>Then, when you dive into a command
(UIForgeStructure.pgn), some commands use
interface and implementation (see in the
second
diagram JavaAddAnnotationCommandImpl
implementing JavaAddAnnotationCommand),
some don't (e.g. NewQualifierCommand). Is
there a reason ?</div>
<div><br>
</div>
<div><br>
</div>
<div>Correct me if I'm wrong, but the way I
see it would be (HowIseeIt.png) :
under AbstractJavaEECommand you have a set
of AbstractValidationCommand, AbstractCDICommand, AbstractJPACommand....
each
implementing PrerequisiteCommandsProvider
(this way, each command sets up its own
pre-requisite). And then,
under AbstractCDICommand you have all
the NewQualifierCommand, NewBeanCommand....</div>
<div><br>
</div>
<div><br>
</div>
<div>What do you think ? Am I the only one
getting a little bit lost ;o)</div>
<div><br>
</div>
<div>What do you think of re-structuring the
class hierarchy ?</div>
<div><br>
</div>
-- <br>
Antonio Goncalves <br>
Software architect and Java Champion<br>
<br>
<a href="http://www.antoniogoncalves.org/" target="_blank">Web site</a> | <a href="http://twitter.com/agoncal" target="_blank">Twitter</a> | <a href="http://www.linkedin.com/in/agoncal" target="_blank">LinkedIn</a> | <a href="http://www.parisjug.org/" target="_blank">Paris JUG</a> | <a href="http://www.devoxx.fr/" target="_blank">Devoxx France</a> </div>
</div>
<br>
<fieldset></fieldset>
<br>
</div>
</div>
<pre>_______________________________________________
forge-dev mailing list
<a href="mailto:forge-dev@lists.jboss.org" target="_blank">forge-dev@lists.jboss.org</a>
<a href="https://lists.jboss.org/mailman/listinfo/forge-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/forge-dev</a></pre>
</blockquote>
<br>
</div>
<br>
_______________________________________________<br>
forge-dev mailing list<br>
<a href="mailto:forge-dev@lists.jboss.org" target="_blank">forge-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/forge-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/forge-dev</a><br>
</blockquote>
</div>
<br>
<br clear="all">
<div><br>
</div>
-- <br>
Antonio Goncalves <br>
Software architect and Java Champion<br>
<br>
<a href="http://www.antoniogoncalves.org/" target="_blank">Web
site</a> | <a href="http://twitter.com/agoncal" target="_blank">Twitter</a> | <a href="http://www.linkedin.com/in/agoncal" target="_blank">LinkedIn</a> | <a href="http://www.parisjug.org/" target="_blank">Paris JUG</a> | <a href="http://www.devoxx.fr/" target="_blank">Devoxx France</a>
</div>
</div>
</div>
<br>
<fieldset></fieldset>
<br>
<pre>_______________________________________________
forge-dev mailing list
<a href="mailto:forge-dev@lists.jboss.org" target="_blank">forge-dev@lists.jboss.org</a>
<a href="https://lists.jboss.org/mailman/listinfo/forge-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/forge-dev</a></pre>
</blockquote>
<br>
</div></div></div>
<br>_______________________________________________<br>
forge-dev mailing list<br>
<a href="mailto:forge-dev@lists.jboss.org">forge-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/forge-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/forge-dev</a><br></blockquote></div><br><br clear="all"><div><br></div>-- <br>Antonio Goncalves <br>Software architect and Java Champion<br><br><a href="http://www.antoniogoncalves.org/" target="_blank">Web site</a> | <a href="http://twitter.com/agoncal" target="_blank">Twitter</a> | <a href="http://www.linkedin.com/in/agoncal" target="_blank">LinkedIn</a> | <a href="http://www.parisjug.org/" target="_blank">Paris JUG</a> | <a href="http://www.devoxx.fr/" target="_blank">Devoxx France</a>
</div>