RED

refs 2255

org.fortiss.tooling.base/trunk/src/org/fortiss/tooling/base/annotation/valueprovider/DerivedAnnotationValueProviderBase.java

@@ -48,20 +48,19 @@ public abstract class DerivedAnnotationValueProviderBase<T extends IDerivedAnnot
 	private List<String> keys = new ArrayList<String>();
 
 	/**
-	 * Constructs a {@link IAnnotationValueProvider} for a pure derived annotations (i.e., an
-	 * {@link IDerivedAnnotation} that does not contain any additional {@link EStructuralFeature}s.
+	 * Constructs a {@link IAnnotationValueProvider} for a pure derived annotation (i.e., an
+	 * {@link IDerivedAnnotation} that does not contain any additional {@link EStructuralFeature}s).
 	 */
 	public DerivedAnnotationValueProviderBase(EClass annotatedSpecificationEClass) {
 		super(annotatedSpecificationEClass);
 		handlesMultipleEstructuralFeatures = false;
 		derivedKey = DEFAULT_KEY;
-
 		keys.add(DEFAULT_KEY);
 	}
 
 	/**
 	 * Constructs a {@link IAnnotationValueProvider} for a pure derived annotations (i.e., an
-	 * {@link IDerivedAnnotation} that does not contain any additional {@link EStructuralFeature}s.
+	 * {@link IDerivedAnnotation} that does not contain any additional {@link EStructuralFeature}s).
 	 */
 	public DerivedAnnotationValueProviderBase(EClass annotatedSpecificationEClass,
 			boolean isEditableByUser) {
@@ -71,7 +70,7 @@ public abstract class DerivedAnnotationValueProviderBase<T extends IDerivedAnnot
 
 	/**
 	 * Constructs a {@link IAnnotationValueProvider} for a derived annotations (i.e., based on an
-	 * {@link IDerivedAnnotation}) that contains additional {@link EStructuralFeature}s.
+	 * {@link IDerivedAnnotation}) that contains additional {@link EStructuralFeature}s).
 	 * 
 	 * @param annotatedSpecificationEClass
 	 *            {@link EClass} implementing the annotation
@@ -136,6 +135,7 @@ public abstract class DerivedAnnotationValueProviderBase<T extends IDerivedAnnot
 	/** {@inheritDoc} */
 	@Override
 	public boolean canEdit(T specification, String instanceKey) {
+		// TODO(VA) B11
 		if(isDerivedKey(instanceKey) &&
 				!isEditableByUser ||
 				(isEditableByUser && (!specification.isUserAnnotatedValuePreferred() && specification

org.fortiss.tooling.base/trunk/src/org/fortiss/tooling/base/annotation/valueprovider/DynamicInstanceAnnotationValueProviderBase.java

@@ -39,7 +39,7 @@ import org.fortiss.tooling.base.model.element.IAnnotatedSpecification;
  * </p>
  * 
  * <p>
- * The underlying {@link EMap} must be declared in the {@link IAnnotatedSpecification}'s meta-model
+ * The underlying {@link EMap} must be declared in the {@link IAnnotatedSpecification}'s metamodel
  * as detailed here: <a href="http://wiki.eclipse.org/EMF/FAQ#How_do_I_create_a_Map_in_EMF.3F">
  * http://wiki.eclipse.org/EMF/FAQ#How_do_I_create_a_Map_in_EMF.3F</a>.
  * </p>
@@ -65,6 +65,9 @@ public abstract class DynamicInstanceAnnotationValueProviderBase<T extends IAnno
 	EFactory valueFactory;
 
 	/**
+	 * TODO(VA) Typo below "that whose". I don't fix myself cause it's also copy-pasted in the next
+	 * comment, so I just let you have a look so that you can check if the copy-pasting is relevant.
+	 * 
 	 * Constructs a multi-instance enabled {@link IAnnotationValueProvider} for a
 	 * {@link IAnnotatedSpecification} that whose storage is implemented by a single
 	 * {@link EAttribute}.
@@ -96,6 +99,7 @@ public abstract class DynamicInstanceAnnotationValueProviderBase<T extends IAnno
 		EMap<String, V> map = super.getAnnotationValue(specification, DEFAULT_KEY);
 
 		return map.get(instanceKey);
+		// TODO(VA) B10 ;-)
 
 	}
 
@@ -110,6 +114,14 @@ public abstract class DynamicInstanceAnnotationValueProviderBase<T extends IAnno
 		EMap<String, V> map = super.getAnnotationValue(specification, DEFAULT_KEY);
 
 		V val = (V)valueFactory.createFromString(valueDataType, value);
+		// TODO(VA) I always find more beautiful to write:
+		// val = val == null ? (V)valueDataType.getDefaultValue() : val;
+		// val = val == null ? (V)valueFactory.createFromString(valueDataType, "0") : val;
+		// because more concise than the 6 above lines, but everybody hates me for that, so do it if
+		// you like it.
+		//
+		// If you don't, you could maybe put the second "if" inside the first one: semantically
+		// clearer.
 		if(val == null) {
 			val = (V)valueDataType.getDefaultValue();
 		}
@@ -127,6 +139,7 @@ public abstract class DynamicInstanceAnnotationValueProviderBase<T extends IAnno
 		if(value instanceof String) {
 			setAnnotationValueFromString((String)value, specification, instanceKey);
 		} else {
+			// TODO(VA) If you like unreadable but concise code, you can inline "map"
 			EMap<String, V> map = super.getAnnotationValue(specification, DEFAULT_KEY);
 			map.put(instanceKey, value);
 		}
@@ -150,14 +163,19 @@ public abstract class DynamicInstanceAnnotationValueProviderBase<T extends IAnno
 			kVMap = super.getAnnotationValue(specification, DEFAULT_KEY);
 		} catch(Exception e) {
 			// Ignore exception
+			// TODO(VA) Theoretically B8, but I'm starting to question this guideline...
 		}
 
 		if(kVMap != null) {
+			// TODO(VA) Slightly more concise than the two lines below:
+			// List<String> rval = new ArrayList<String>(kVMap.keySet());
 			List<String> rval = new ArrayList<String>();
 			rval.addAll(kVMap.keySet());
+			// TODO(VA) B17
 			Collections.sort(rval);
 			return rval;
 		}
+		// TODO(VA) B17
 		return Collections.emptyList();
 	}