This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch WW-5233-tiles
in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/WW-5233-tiles by this push:
new e9cad7ab3 WW-5233 Addresses a few code smells
e9cad7ab3 is described below
commit e9cad7ab3cbb3fe5e447015b2101b67b85f00b76
Author: Lukasz Lenart <lukaszlen...@apache.org>
AuthorDate: Mon Oct 3 08:09:34 2022 +0200
WW-5233 Addresses a few code smells
---
.../src/main/java/org/apache/tiles/api/Attribute.java | 18 ++++++------------
.../org/apache/tiles/api/BasicAttributeContext.java | 4 ++--
.../main/java/org/apache/tiles/api/ListAttribute.java | 9 ++++-----
.../test/java/org/apache/tiles/api/AttributeTest.java | 2 +-
.../java/org/apache/tiles/api/ListAttributeTest.java | 2 +-
.../request/collection/HeaderValuesCollectionTest.java | 4 ++--
.../collection/HeaderValuesMapEntrySetTest.java | 4 ++--
.../apache/tiles/request/collection/KeySetTest.java | 2 +-
.../ReadOnlyEnumerationMapValuesCollectionTest.java | 6 ++++--
9 files changed, 23 insertions(+), 28 deletions(-)
diff --git a/plugins/tiles/src/main/java/org/apache/tiles/api/Attribute.java
b/plugins/tiles/src/main/java/org/apache/tiles/api/Attribute.java
index 69209e5d4..65be6537a 100644
--- a/plugins/tiles/src/main/java/org/apache/tiles/api/Attribute.java
+++ b/plugins/tiles/src/main/java/org/apache/tiles/api/Attribute.java
@@ -22,7 +22,6 @@ package org.apache.tiles.api;
import com.opensymphony.xwork2.util.TextParseUtil;
import org.apache.tiles.request.Request;
-import java.io.Serializable;
import java.util.Iterator;
import java.util.Objects;
import java.util.Set;
@@ -30,7 +29,7 @@ import java.util.Set;
/**
* Common implementation of attribute definition.
*/
-public class Attribute implements Serializable, Cloneable {
+public class Attribute {
/**
* The name of the template renderer.
@@ -42,20 +41,19 @@ public class Attribute implements Serializable, Cloneable {
*
* @since 2.0.6
*/
- protected Set<String> roles = null;
+ private Set<String> roles = null;
/**
* The value of the attribute.
*/
- protected Object value = null;
+ private Object value = null;
/**
- * The expression to evaluate. Ignored if {@link #value} is not
- * <code>null</code>.
+ * The expression to evaluate. Ignored if {@link #value} is not
<code>null</code>.
*
* @since 2.2.0
*/
- protected Expression expressionObject = null;
+ private Expression expressionObject = null;
/**
* The renderer name of the attribute. Default names are
<code>string</code>,
@@ -362,11 +360,7 @@ public class Attribute implements Serializable, Cloneable {
+ Objects.hashCode(roles) + Objects.hashCode(expressionObject);
}
- /**
- * {@inheritDoc}
- */
- @Override
- public Attribute clone() {
+ public Attribute copy() {
return new Attribute(this);
}
}
diff --git
a/plugins/tiles/src/main/java/org/apache/tiles/api/BasicAttributeContext.java
b/plugins/tiles/src/main/java/org/apache/tiles/api/BasicAttributeContext.java
index 9de064d6b..cd12ea860 100644
---
a/plugins/tiles/src/main/java/org/apache/tiles/api/BasicAttributeContext.java
+++
b/plugins/tiles/src/main/java/org/apache/tiles/api/BasicAttributeContext.java
@@ -232,7 +232,7 @@ public class BasicAttributeContext implements
AttributeContext, Serializable {
/**
* Add all attributes to this context.
- * Copies all of the mappings from the specified attribute map to this
context.
+ * Copies all the mappings from the specified attribute map to this
context.
* New attribute mappings will replace any mappings that this context had
for any of the keys
* currently in the specified attribute map.
*
@@ -454,7 +454,7 @@ public class BasicAttributeContext implements
AttributeContext, Serializable {
for (Map.Entry<String, Attribute> entry : attributes.entrySet()) {
Attribute toCopy = entry.getValue();
if (toCopy != null) {
- retValue.put(entry.getKey(), toCopy.clone());
+ retValue.put(entry.getKey(), toCopy.copy());
}
}
return retValue;
diff --git
a/plugins/tiles/src/main/java/org/apache/tiles/api/ListAttribute.java
b/plugins/tiles/src/main/java/org/apache/tiles/api/ListAttribute.java
index 227380af0..67b329443 100644
--- a/plugins/tiles/src/main/java/org/apache/tiles/api/ListAttribute.java
+++ b/plugins/tiles/src/main/java/org/apache/tiles/api/ListAttribute.java
@@ -61,7 +61,7 @@ public class ListAttribute extends Attribute {
List<Attribute> attributes = new
ArrayList<>(attributesToCopy.size());
for (Attribute attribute : attributesToCopy) {
if (attribute != null) {
- attributes.add(attribute.clone());
+ attributes.add(attribute.copy());
} else {
attributes.add(null);
}
@@ -135,11 +135,10 @@ public class ListAttribute extends Attribute {
* @param parent The parent list attribute.
* @since 2.1.0
*/
- @SuppressWarnings("unchecked")
public void inherit(ListAttribute parent) {
List<Attribute> tempList = new ArrayList<>();
- tempList.addAll((List<Attribute>) parent.value);
- tempList.addAll((List<Attribute>) value);
+ tempList.addAll(parent.getValue());
+ tempList.addAll(getValue());
setValue(tempList);
}
@@ -164,7 +163,7 @@ public class ListAttribute extends Attribute {
/** {@inheritDoc} */
@Override
- public ListAttribute clone() {
+ public ListAttribute copy() {
return new ListAttribute(this);
}
}
diff --git
a/plugins/tiles/src/test/java/org/apache/tiles/api/AttributeTest.java
b/plugins/tiles/src/test/java/org/apache/tiles/api/AttributeTest.java
index 5c46b42e0..703dde92b 100644
--- a/plugins/tiles/src/test/java/org/apache/tiles/api/AttributeTest.java
+++ b/plugins/tiles/src/test/java/org/apache/tiles/api/AttributeTest.java
@@ -225,7 +225,7 @@ public class AttributeTest {
public void testClone() {
Expression expression = new Expression("my.expression", "MYLANG");
Attribute attribute = new Attribute("my.value", expression,
"role1,role2", "myrenderer");
- attribute = attribute.clone();
+ attribute = attribute.copy();
assertEquals("my.value", attribute.getValue());
assertEquals("myrenderer", attribute.getRenderer());
Set<String> roles = new HashSet<>();
diff --git
a/plugins/tiles/src/test/java/org/apache/tiles/api/ListAttributeTest.java
b/plugins/tiles/src/test/java/org/apache/tiles/api/ListAttributeTest.java
index 582f041e8..baa41fd7a 100644
--- a/plugins/tiles/src/test/java/org/apache/tiles/api/ListAttributeTest.java
+++ b/plugins/tiles/src/test/java/org/apache/tiles/api/ListAttributeTest.java
@@ -103,7 +103,7 @@ public class ListAttributeTest {
list.add(new Attribute("value2"));
attribute.setValue(list);
attribute.setInherit(true);
- ListAttribute toCheck = attribute.clone();
+ ListAttribute toCheck = attribute.copy();
assertEquals(attribute, toCheck);
}
}
diff --git
a/plugins/tiles/src/test/java/org/apache/tiles/request/collection/HeaderValuesCollectionTest.java
b/plugins/tiles/src/test/java/org/apache/tiles/request/collection/HeaderValuesCollectionTest.java
index 8a4069171..36f9bd2e0 100644
---
a/plugins/tiles/src/test/java/org/apache/tiles/request/collection/HeaderValuesCollectionTest.java
+++
b/plugins/tiles/src/test/java/org/apache/tiles/request/collection/HeaderValuesCollectionTest.java
@@ -42,7 +42,6 @@ import static org.junit.Assert.assertTrue;
*/
public class HeaderValuesCollectionTest {
-
/**
* The extractor to use.
*/
@@ -245,9 +244,10 @@ public class HeaderValuesCollectionTest {
expect(extractor.getKeys()).andReturn(keys);
expect(keys.hasMoreElements()).andReturn(true);
+ expect(keys.hasMoreElements()).andReturn(true);
expect(keys.nextElement()).andReturn("two");
-
expect(extractor.getValues("two")).andReturn(values2);
+
expect(values2.hasMoreElements()).andReturn(true);
expect(values2.nextElement()).andReturn("value2");
expect(values2.hasMoreElements()).andReturn(true);
diff --git
a/plugins/tiles/src/test/java/org/apache/tiles/request/collection/HeaderValuesMapEntrySetTest.java
b/plugins/tiles/src/test/java/org/apache/tiles/request/collection/HeaderValuesMapEntrySetTest.java
index 344151c52..77218414d 100644
---
a/plugins/tiles/src/test/java/org/apache/tiles/request/collection/HeaderValuesMapEntrySetTest.java
+++
b/plugins/tiles/src/test/java/org/apache/tiles/request/collection/HeaderValuesMapEntrySetTest.java
@@ -180,6 +180,7 @@ public class HeaderValuesMapEntrySetTest {
expect(extractor.getKeys()).andReturn(keys);
expect(keys.hasMoreElements()).andReturn(true);
+ expect(keys.hasMoreElements()).andReturn(true);
expect(keys.nextElement()).andReturn("two");
expect(extractor.getValues("two")).andReturn(values2);
@@ -192,8 +193,7 @@ public class HeaderValuesMapEntrySetTest {
replay(extractor, keys, values2);
Iterator<Map.Entry<String, String[]>> entryIt = entrySet.iterator();
assertTrue(entryIt.hasNext());
- MapEntryArrayValues<String, String> entry = new MapEntryArrayValues<>(
- "two", new String[]{"value2", "value3"}, false);
+ MapEntryArrayValues<String, String> entry = new
MapEntryArrayValues<>("two", new String[]{"value2", "value3"}, false);
assertEquals(entry, entryIt.next());
verify(extractor, keys, values2);
}
diff --git
a/plugins/tiles/src/test/java/org/apache/tiles/request/collection/KeySetTest.java
b/plugins/tiles/src/test/java/org/apache/tiles/request/collection/KeySetTest.java
index f8fe60a31..452bb5123 100644
---
a/plugins/tiles/src/test/java/org/apache/tiles/request/collection/KeySetTest.java
+++
b/plugins/tiles/src/test/java/org/apache/tiles/request/collection/KeySetTest.java
@@ -42,7 +42,6 @@ import static org.junit.Assert.assertTrue;
*/
public class KeySetTest {
-
/**
* The extractor to use.
*/
@@ -170,6 +169,7 @@ public class KeySetTest {
expect(extractor.getKeys()).andReturn(keys);
expect(keys.hasMoreElements()).andReturn(true);
+ expect(keys.hasMoreElements()).andReturn(true);
expect(keys.nextElement()).andReturn("two");
replay(extractor, keys, values2);
diff --git
a/plugins/tiles/src/test/java/org/apache/tiles/request/collection/ReadOnlyEnumerationMapValuesCollectionTest.java
b/plugins/tiles/src/test/java/org/apache/tiles/request/collection/ReadOnlyEnumerationMapValuesCollectionTest.java
index 4a9862f82..b37dbdc03 100644
---
a/plugins/tiles/src/test/java/org/apache/tiles/request/collection/ReadOnlyEnumerationMapValuesCollectionTest.java
+++
b/plugins/tiles/src/test/java/org/apache/tiles/request/collection/ReadOnlyEnumerationMapValuesCollectionTest.java
@@ -41,6 +41,7 @@ import static org.junit.Assert.assertTrue;
* Tests {@link ReadOnlyEnumerationMap#values()}.
*/
public class ReadOnlyEnumerationMapValuesCollectionTest {
+
/**
* The extractor to use.
*/
@@ -188,6 +189,7 @@ public class ReadOnlyEnumerationMapValuesCollectionTest {
expect(extractor.getKeys()).andReturn(keys);
expect(keys.hasMoreElements()).andReturn(true);
+ expect(keys.hasMoreElements()).andReturn(true);
expect(keys.nextElement()).andReturn("two");
expect(extractor.getValue("two")).andReturn(2);
@@ -276,7 +278,7 @@ public class ReadOnlyEnumerationMapValuesCollectionTest {
expect(extractor.getValue("one")).andReturn(1);
expect(extractor.getValue("two")).andReturn(2);
- Integer[] entryArray = new Integer[] {1, 2};
+ Integer[] entryArray = new Integer[]{1, 2};
replay(extractor, keys);
assertArrayEquals(entryArray, coll.toArray());
@@ -300,7 +302,7 @@ public class ReadOnlyEnumerationMapValuesCollectionTest {
expect(extractor.getValue("one")).andReturn(1);
expect(extractor.getValue("two")).andReturn(2);
- Integer[] entryArray = new Integer[] {1, 2};
+ Integer[] entryArray = new Integer[]{1, 2};
replay(extractor, keys);
Integer[] realArray = new Integer[2];
