RED

refs 2255

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

@@ -50,6 +50,10 @@ public abstract class ValueProviderBase<T extends IAnnotatedSpecification> imple
 	protected final EClass annotatedSpecificationEClass;
 
 	/**
+	 * TODO(VA) Probably obvious for somebody who knows already how to deal with annotations but not
+	 * for a newcomer, so take this remark into account only if you think it's valuable: what is an
+	 * instanceKey?
+	 * 
 	 * Map: instanceKey -> Fixed input choice to be offered in a combo box. The semantics of the map
 	 * value is as follows:
 	 * <ul>
@@ -58,7 +62,7 @@ public abstract class ValueProviderBase<T extends IAnnotatedSpecification> imple
 	 * <li> {@code !fixedInputChoice[key].isEmpty()} -> provide a fixed set of choices in an input
 	 * box.
 	 * <li> {@code fixedInputChoice[key].isEmpty()} -> dynamically manage the set of provided values,
-	 * i.e. the combo box is editable and enables the user to add new values and remove existing
+	 * i.e., the combo box is editable and enables the user to add new values and remove existing
 	 * choices (by removing all instances of them).</li>
 	 * <li>For enum types, the set of inputs is automatically initialized from the underlying
 	 * {@link EEnum} type definition.</li>
@@ -86,7 +90,7 @@ public abstract class ValueProviderBase<T extends IAnnotatedSpecification> imple
 	/** {@inheritDoc} */
 	@Override
 	public boolean setAnnotationName(String name, T specification) {
-		// Renaming this annotation is not supported.
+		// Renaming this annotation is not supported by default.
 		return false;
 	}
 
@@ -168,6 +172,7 @@ public abstract class ValueProviderBase<T extends IAnnotatedSpecification> imple
 	@Override
 	public T getAnnotatedSpecificationForModelElement(IModelElement element) throws Exception {
 
+		// TODO(VA,B11)
 		T rval =
 				(T)pickFirstInstanceOf(
 						(Class<IAnnotatedSpecification>)annotatedSpecificationEClass
@@ -178,6 +183,7 @@ public abstract class ValueProviderBase<T extends IAnnotatedSpecification> imple
 			return rval;
 		}
 
+		// TODO(VA,B11)
 		// Create annotation
 		T specification =
 				(T)annotatedSpecificationEClass.getEPackage().getEFactoryInstance()
@@ -242,9 +248,17 @@ public abstract class ValueProviderBase<T extends IAnnotatedSpecification> imple
 	 * Updates the current input choice, and returns the actual value. Called from
 	 * {@link #setAnnotationValue(String, IAnnotatedSpecification)}. In particular, this
 	 * method handles the removal of input choices from if these are dynamically managed.
+	 * TODO(VA) "from" what?
+	 * TODO(VA) What's actually the use of "value" here? First it's confusing to provide a
+	 * value in a "get***" function. Second it's used only in the very first condition and in a very
+	 * limited way. Is it provided as a facility to overloading methods? In any case, deserves an
+	 * explanation in comment, I think.
+	 * I must say overall I don't really get what this method is doing, but I'm not familiar with
+	 * annotations.
 	 */
 	protected String getAnnotationValueAndUpdateInputChoice(String value, T specification,
 			String instanceKey) throws Exception {
+		// TODO(VA,B11)
 		if((value == null || value.isEmpty()) && currentInputChoiceMap.get(instanceKey) != null &&
 				!currentInputChoiceMap.get(instanceKey).isEmpty()) {
 			// Only one entry left: do not remove
@@ -254,6 +268,8 @@ public abstract class ValueProviderBase<T extends IAnnotatedSpecification> imple
 
 			// 1st entry is to be removed: Choose 2nd entry as new value
 			String currentValue = getAnnotationValue(specification);
+			// TODO(VA) currentInputChoiceMap.get(instanceKey).iterator().next() could be factorized
+			// in the next three lines
 			if(currentInputChoiceMap.get(instanceKey).iterator().next().equals(currentValue)) {
 				Iterator<String> iter = currentInputChoiceMap.get(instanceKey).iterator();
 				iter.next();
@@ -264,6 +280,9 @@ public abstract class ValueProviderBase<T extends IAnnotatedSpecification> imple
 
 			// Otherwise, return the entry just before the entry to be removed.
 			String rval = null;
+			// TODO(VA) More concise:
+			// for(String s : currentInputChoiceMap.get(instanceKey))
+			// instead of the three following lines.
 			for(Iterator<String> iter = currentInputChoiceMap.get(instanceKey).iterator(); iter
 					.hasNext();) {
 				String s = iter.next();