RED

refs 2490

org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/CommandStackService.java

@@ -73,6 +73,10 @@ public class CommandStackService implements ICommandStackService, IIntrospective
 		return getIntrospectionLabel() +
 				"\n\nThis service command stack used to make changes to models." +
 				"\nIt also handles the undo/redo feature and the dirty state of models.";
+		// TODO(VA) Description could be more detailed IMHO, at least if we aim it to substitute the
+		// documentation of the kernel: why do we need a service to make changes to
+		// model? How is nesting of commands handled by the undo/redo?
+		// Also the first sentence is not really English.
 	}
 
 	/** {@inheritDoc} */

org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/DummyTopLevelElement.java

@@ -41,8 +41,7 @@ import org.fortiss.tooling.kernel.service.IPersistencyService;
 
 /**
  * The dummy top-level element is used to load models into the {@link IPersistencyService} without
- * an actual underlying storage. This is
- * primarily intended to load models for unit testing.
+ * an actual underlying storage. This is primarily intended to load models for unit testing.
  * 
  * @author hoelzl
  * @author $Author$
@@ -84,6 +83,7 @@ final class DummyTopLevelElement implements ITopLevelElement, CommandStackListen
 	}
 
 	/** Runs the given runnable as a command. */
+	// TODO(VA) Why not just /** {@inheritDoc} */ as comment?
 	@Override
 	public void runAsCommand(final Runnable runnable) {
 		transactionalCommandStack.execute(new AbstractCommand() {
@@ -106,6 +106,7 @@ final class DummyTopLevelElement implements ITopLevelElement, CommandStackListen
 	}
 
 	/** Runs the given runnable as a command that does not make the model dirty. */
+	// TODO(VA) Why not just /** {@inheritDoc} */ as comment?
 	@Override
 	public void runAsNonDirtyingCommand(final Runnable runnable) {
 		try {

org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/ElementCompositorService.java

@@ -88,6 +88,8 @@ public final class ElementCompositorService extends
 				"\nis added to a parent component). Most often contextual data (e.g. the mouse pointer position in a diagram) is used " +
 				"\nto add specific model values during the composition procedure." +
 				"\n\nThe service extension point is '" + EXTENSION_POINT_NAME + "'.";
+		// TODO(VA) I think it would also help the newcomer to know why there is a need for a
+		// service and why they should avoid composing elements manually
 	}
 
 	/** {@inheritDoc} */

org.fortiss.tooling.kernel/trunk/src/org/fortiss/tooling/kernel/internal/LibraryPrototypeProvider.java

@@ -62,6 +62,9 @@ public class LibraryPrototypeProvider extends PrototypeProviderBase {
 					((LibraryService)libs).getRegisteredHandlers(libElem.getWrappedElement()
 							.getClass());
 			// LibraryService#getRegisteredHandlers() returns at most first handler
+			// TODO(VA) Comment above not so clear: "at most first" not really English.
+			// "at most one and when one the first"?
+			// If so I guess this is actually the comment for the get(0) below? Not really clear.
 			if(!handlers.isEmpty()) {
 				ILibraryElementHandler<EObject> firstHandler = handlers.get(0);