Set to yellow. Review item in EObjectActionBase

refs 451

org.fortiss.tooling.kernel.ui/trunk/src/org/fortiss/tooling/kernel/ui/extension/base/ContextMenuSubMenuContributorBase.java

@@ -28,9 +28,9 @@ import org.eclipse.jface.action.IContributionItem;
 import org.eclipse.jface.action.IMenuManager;
 import org.eclipse.jface.action.MenuManager;
 import org.eclipse.jface.resource.ImageDescriptor;
-import org.fortiss.tooling.kernel.model.INamedElement;
+import org.eclipse.ui.actions.ActionFactory;
 import org.fortiss.tooling.kernel.ui.extension.IContextMenuContributor;
-import org.fortiss.tooling.kernel.ui.extension.base.MenuActionBase.ActionFactory;
+import org.fortiss.tooling.kernel.ui.extension.base.EObjectActionBase.EObjectActionFactory;
 import org.fortiss.tooling.kernel.ui.extension.data.ContextMenuContextProvider;
 import org.fortiss.tooling.kernel.ui.service.IContextMenuService;
 
@@ -39,11 +39,9 @@ import org.fortiss.tooling.kernel.ui.service.IContextMenuService;
  * {@link IMenuManager} with actions provided by actions factories
  * {@link ActionFactory} returned by {@link #getActionFactories()}.
  * 
- * TODO @review This class implements its interface with default values, e.g.
- * getMenuSectionID. This should be documented here. See ModelElementHandlerBase
- * for example.
- * <p>
- * TODO @review What is or where is <T> here?
+ * The class implements interface methods {@link #getMenuIcon()} and
+ * {@link #getMenuId()} returning {@code null} by default. Classes providing an
+ * own icon or id should override them.
  * 
  * The generic type parameter {@code T} indicated the type of selection
  * elements.
@@ -51,27 +49,21 @@ import org.fortiss.tooling.kernel.ui.service.IContextMenuService;
  * @author trachtenherz
  * @author $Author: hoelzl $
  * @version $Rev: 18709 $
- * @ConQAT.Rating YELLOW Hash: 839C689F53DDF4A697F294BBD4DB2B74
+ * @ConQAT.Rating YELLOW Hash: DE788BDCC0598D12129DAA00E06DDE3E
  */
-public abstract class ContextMenuSubMenuContributorBase implements
-		IContextMenuContributor {
-
-	/** Constructor. */
-	public ContextMenuSubMenuContributorBase() {
-		// TODO @review unnecessary contructor
-		super();
-	}
+public abstract class ContextMenuSubMenuContributorBase<T extends EObject>
+		implements IContextMenuContributor {
 
 	/** The name of the sub-menu */
 	protected abstract String getMenuName();
 
 	/** Returns the action factories for the actions to be shown in the menu */
-	protected abstract Collection<ActionFactory<INamedElement>> getActionFactories();
+	protected abstract Collection<EObjectActionFactory<? super T>> getActionFactories();
 
 	/** Creates action instances for the given selection. */
-	protected Collection<Action> createActionsForElement(INamedElement selection) {
+	protected Collection<Action> createActionsForElement(T selection) {
 		ArrayList<Action> list = new ArrayList<Action>();
-		for (ActionFactory<INamedElement> factory : getActionFactories()) {
+		for (EObjectActionFactory<? super T> factory : getActionFactories()) {
 			list.add(factory.createAction(selection));
 		}
 		return list;
@@ -100,14 +92,14 @@ public abstract class ContextMenuSubMenuContributorBase implements
 	}
 
 	/** {@inheritDoc} */
+	@SuppressWarnings("unchecked")
 	@Override
 	public final List<IContributionItem> getContributedItems(EObject selection,
 			ContextMenuContextProvider contextProvider) {
-		if (selection instanceof INamedElement
-				&& acceptSelection((INamedElement) selection, contextProvider)) {
+		if (acceptSelection(selection, contextProvider)) {
 			List<IContributionItem> contributionItems = new ArrayList<IContributionItem>();
 			contributionItems
-					.add(createMenu(createActionsForElement((INamedElement) selection)));
+					.add(createMenu(createActionsForElement((T) selection)));
 			return contributionItems;
 		}
 		return Collections.emptyList();
@@ -119,7 +111,10 @@ public abstract class ContextMenuSubMenuContributorBase implements
 		return IContextMenuService.BOTTOM_MOST_MENU_SECTION_ID;
 	}
 
-	/** Returns {@code true} if the selected element is acceptable. */
-	protected abstract boolean acceptSelection(INamedElement selection,
+	/**
+	 * Returns {@code true} if the selected element is acceptable. This method
+	 * must also ensure that {@code selection} is an instance of {@code T}
+	 */
+	protected abstract boolean acceptSelection(EObject selection,
 			ContextMenuContextProvider contextProvider);
 }
\ No newline at end of file

org.fortiss.tooling.kernel.ui/trunk/src/org/fortiss/tooling/kernel/ui/extension/base/EObjectActionBase.java

@@ -27,10 +27,16 @@ import org.eclipse.jface.resource.ImageDescriptor;
  * @author hoelzlf
  * @author $Author$
  * @version $Rev$
- * @ConQAT.Rating GREEN Hash: 9995CEA8009A5730AC380E5017A51C5F
+ * @ConQAT.Rating YELLOW Hash: 4C545F6468D6051419AA91C0E3774061
  */
 public abstract class EObjectActionBase<T extends EObject> extends Action {
 
+	/** Factory for the action instances used for this menu */
+	public static interface EObjectActionFactory<T extends EObject> {
+		/** Creates and returns an action instance for the given target. */
+		EObjectActionBase<T> createAction(T target);
+	}
+
 	/** Stores the target element. */
 	private T target;
 
@@ -39,6 +45,22 @@ public abstract class EObjectActionBase<T extends EObject> extends Action {
 		super(text, image);
 	}
 
+	/** Constructor. */
+	public EObjectActionBase(String text) {
+		this(text, null);
+	}
+
+	/** Constructor. */
+	public EObjectActionBase(T target, String text, ImageDescriptor image) {
+		this(text, image);
+		setTarget(target);
+	}
+
+	/** Constructor. */
+	public EObjectActionBase(T target, String text) {
+		this(target, text, null);
+	}
+
 	/** Sets the target object. */
 	public void setTarget(T target) {
 		this.target = target;
@@ -50,6 +72,12 @@ public abstract class EObjectActionBase<T extends EObject> extends Action {
 	}
 
 	/** Refreshes the action's target and enabled state. */
+	// TODO @review DT: should a user take setTarget or refresh for a new
+	// element?
+	// Should a constructor take setTarget or refresh? should there be a refresh
+	// method for the currently set element, i. e. refresh(). Possibility:
+	// refresh() for current element and refresh(element){setTarget(element);
+	// refresh()}.
 	public final void refresh(T element) {
 		setTarget(element);
 		if (element == null) {