Remove usage of the getTag and setTag methods

CHANGELOG.md

@@ -62,6 +62,8 @@
 ## 1.5.20
 
 ### Enhancements
+* Removed usage of deprecated APIs from the framework and examples. Including the following:
+    - Removed usage of `WComponents::getTag` and `WComponents::setTag` from `WDefinitionList`.
 
 * Removed usage of deprecated APIs from the framework and examples. Including the following:
     - Replaced usage of `WBeanContainer` with `WContainer`. #1733

wcomponents-core/src/main/java/com/github/bordertech/wcomponents/WDefinitionList.java

@@ -2,9 +2,9 @@
 
 import com.github.bordertech.wcomponents.util.Duplet;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
+import java.util.Optional;
+import java.util.stream.Collectors;
 
 /**
  * <p>
@@ -16,6 +16,8 @@
 public class WDefinitionList extends AbstractNamingContextContainer implements AjaxTarget,
 		SubordinateTarget, Marginable {
 
+	private static final String TERM_ATTRIBUTE = "WDefinitionList.term";
+
 	/**
 	 * The layout options.
 	 */
@@ -100,24 +102,27 @@ public Margin getMargin() {
 	 * @param data the term data.
 	 */
 	public void addTerm(final String term, final WComponent... data) {
-		for (WComponent component : data) {
-			if (component != null) {
-				content.add(component, term);
-			}
-		}
-
-		// If the term doesn't exist, we may need to add a dummy component
-		if (getComponentsForTerm(term).isEmpty()) {
-			content.add(new DefaultWComponent(), term);
-		}
+		getOrAddTermContainer(term)
+			.ifPresent(container -> {
+				for (WComponent datum : data) {
+					datum.setTag(term);
+					container.add(datum);
+				}
+			});
 	}
 
 	/**
 	 * {@inheritDoc}
 	 */
 	@Override
 	public void remove(final WComponent child) {
-		content.remove(child);
+		getTermContainers()
+			.stream()
+			.forEach(container -> {
+				if (container.getChildren().contains(child)) {
+					container.remove(child);
+				}
+			});
 	}
 
 	/**
@@ -126,9 +131,7 @@ public void remove(final WComponent child) {
 	 * @param term the term to remove.
 	 */
 	public void removeTerm(final String term) {
-		for (WComponent child : getComponentsForTerm(term)) {
-			content.remove(child);
-		}
+		getTermContainer(term).ifPresent(container -> content.remove(container));
 	}
 
 	/**
@@ -137,53 +140,42 @@ public void removeTerm(final String term) {
 	 * @return a list of this definition list's children grouped by their terms.
 	 */
 	public List<Duplet<String, ArrayList<WComponent>>> getTerms() {
-		Map<String, Duplet<String, ArrayList<WComponent>>> componentsByTerm = new HashMap<>();
-		List<Duplet<String, ArrayList<WComponent>>> result = new ArrayList<>();
+		return getTermContainers()
+				.stream()
+				.map(container ->
+					new Duplet<>((String) container.getAttribute(TERM_ATTRIBUTE),
+						new ArrayList<>(container.getChildren())))
+				.collect(Collectors.toList());
+	}
 
-		List<WComponent> childList = content.getComponentModel().getChildren();
+	private List<WContainer> getTermContainers() {
+		return content
+			.getChildren()
+			.stream()
+			.filter(child -> (child instanceof WContainer) && ((WContainer) child).getAttribute(TERM_ATTRIBUTE) != null)
+			.map(child -> (WContainer) child)
+			.collect(Collectors.toList());
+	}
 
-		if (childList != null) {
-			for (int i = 0; i < childList.size(); i++) {
-				WComponent child = childList.get(i);
-				String term = child.getTag();
+	private Optional<WContainer> getOrAddTermContainer(final String term) {
 
-				Duplet<String, ArrayList<WComponent>> termComponents = componentsByTerm.get(term);
+		final Optional<WContainer> container = getTermContainer(term);
 
-				if (termComponents == null) {
-					termComponents = new Duplet<>(term,
-							new ArrayList<WComponent>());
-					componentsByTerm.put(term, termComponents);
-					result.add(termComponents);
-				}
-
-				termComponents.getSecond().add(child);
-			}
+		if (term != null && !container.isPresent()) {
+			WContainer newContainer = new WContainer();
+			newContainer.setAttribute(TERM_ATTRIBUTE, term);
+			content.add(newContainer);
+			return Optional.of(newContainer);
 		}
 
-		return result;
+		return container;
 	}
 
-	/**
-	 * Retrieves the components for the given term.
-	 *
-	 * @param term the term of the children to be retrieved.
-	 * @return the child components for the given term, may be empty.
-	 */
-	private List<WComponent> getComponentsForTerm(final String term) {
-		List<WComponent> childList = content.getComponentModel().getChildren();
-		List<WComponent> result = new ArrayList<>();
-
-		if (childList != null) {
-			for (int i = 0; i < childList.size(); i++) {
-				WComponent child = childList.get(i);
-
-				if (term.equals(child.getTag())) {
-					result.add(child);
-				}
-			}
-		}
-
-		return result;
+	private Optional<WContainer> getTermContainer(final String term) {
+		return getTermContainers()
+				.stream()
+				.filter(container -> term != null && term.equals(container.getAttribute(TERM_ATTRIBUTE)))
+				.findFirst();
 	}
 
 	/**

wcomponents-core/src/test/java/com/github/bordertech/wcomponents/WDefinitionList_Test.java

@@ -46,15 +46,21 @@ public void testAddTerm() {
 		WComponent term2data3 = new DefaultWComponent();
 
 		// Test addition of term with no component
+		Assert.assertTrue("There should not be any terms", list.getTerms().isEmpty());
+
 		list.addTerm(term1);
-		Container termContainer = (Container) list.getChildAt(0);
-		Assert.assertEquals("Term1 should have been added", 1, termContainer.getChildCount());
-		Assert.assertEquals("Incorrect value for Term1", term1, termContainer.getChildAt(0).getTag());
+		Assert.assertEquals("There should have been 1 term added", 1, list.getTerms().size());
+		Assert.assertEquals("The first term's name is incorrect", term1, list.getTerms().get(0).getFirst());
+		Assert.assertTrue("The first term should not have any components", list.getTerms().get(0).getSecond().isEmpty());
 
 		// Test addition of term with multiple components
 		list.addTerm(term2, term2data1, term2data2);
 		list.addTerm(term2, term2data3);
 
+		Assert.assertEquals("There should have been 2 terms added", 2, list.getTerms().size());
+		Assert.assertEquals("The second term's name is incorrect", term2, list.getTerms().get(1).getFirst());
+		Assert.assertEquals("The second term should have 3 components", 3, list.getTerms().get(1).getSecond().size());
+
 		Assert.assertEquals("Incorrect term for component 1", term2, term2data1.getTag());
 		Assert.assertEquals("Incorrect term for component 2", term2, term2data2.getTag());
 		Assert.assertEquals("Incorrect term for component 3", term2, term2data3.getTag());
@@ -72,12 +78,30 @@ public void testRemoveTerm() {
 
 		list.addTerm(term1);
 		list.addTerm(term2, term2data1, term2data2);
-		Container termContainer = (Container) list.getChildAt(0);
-
-		Assert.assertEquals("Incorrect term data", 3, termContainer.getChildren().size());
+		Assert.assertEquals("There should have been 2 terms added", 2, list.getTerms().size());
 
 		list.removeTerm(term2);
-		Assert.assertEquals("Incorrect term data", 1, termContainer.getChildCount());
-		Assert.assertEquals("Incorrect value for Term1", term1, termContainer.getChildAt(0).getTag());
+		Assert.assertEquals("There should be only 1 term remaining", 1, list.getTerms().size());
+
+		Assert.assertEquals("Incorrect tag for component 1", term2, term2data1.getTag());
+		Assert.assertEquals("Incorrect tag for component 2", term2, term2data2.getTag());
+	}
+
+	@Test
+	public void testRemoveComponent() {
+		WDefinitionList list = new WDefinitionList();
+
+		String term1 = "WDefinitionList_Test.testRemoveComponent.term1";
+
+		WComponent data1 = new DefaultWComponent();
+		WComponent data2 = new DefaultWComponent();
+
+		list.addTerm(term1, data1, data2);
+		Assert.assertEquals("There should have been 1 terms added", 1, list.getTerms().size());
+		Assert.assertEquals("There should have been 2 components added", 2, list.getTerms().get(0).getSecond().size());
+
+		list.remove(data2);
+
+		Assert.assertEquals("There should be only 1 component remaining", 1, list.getTerms().get(0).getSecond().size());
 	}
 }