From 47169d5907cf969f8b8e2a4265821886c7a4fe5b Mon Sep 17 00:00:00 2001 From: Hallvard Traetteberg <hal@ntnu.no> Date: Thu, 29 Oct 2020 11:31:38 +0100 Subject: [PATCH] antipatterns --- antipatterns/.gitignore | 3 ++ antipatterns/pom.xml | 47 +++++++++++++++++++ .../DataClassWithEffectivelyFinalFields.java | 20 ++++++++ .../DataClassWithLittleLogic.java | 35 ++++++++++++++ .../antipatterns/DataClassWithValidation.java | 26 ++++++++++ .../antipatterns/NonEncapsulatedList.java | 21 +++++++++ .../DataClassWithLittleLogicTest.java | 32 +++++++++++++ .../DataClassWithValidationTest.java | 34 ++++++++++++++ 8 files changed, 218 insertions(+) create mode 100644 antipatterns/.gitignore create mode 100644 antipatterns/pom.xml create mode 100644 antipatterns/src/main/java/antipatterns/DataClassWithEffectivelyFinalFields.java create mode 100644 antipatterns/src/main/java/antipatterns/DataClassWithLittleLogic.java create mode 100644 antipatterns/src/main/java/antipatterns/DataClassWithValidation.java create mode 100644 antipatterns/src/main/java/antipatterns/NonEncapsulatedList.java create mode 100644 antipatterns/src/test/java/antipatterns/DataClassWithLittleLogicTest.java create mode 100644 antipatterns/src/test/java/antipatterns/DataClassWithValidationTest.java diff --git a/antipatterns/.gitignore b/antipatterns/.gitignore new file mode 100644 index 0000000..9a89c84 --- /dev/null +++ b/antipatterns/.gitignore @@ -0,0 +1,3 @@ +target/ +.classpath +.project diff --git a/antipatterns/pom.xml b/antipatterns/pom.xml new file mode 100644 index 0000000..c7dfe47 --- /dev/null +++ b/antipatterns/pom.xml @@ -0,0 +1,47 @@ +<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd"> + <modelVersion>4.0.0</modelVersion> + <groupId>it1901</groupId> + <artifactId>antipatterns</artifactId> + <version>0.0.1-SNAPSHOT</version> + + <dependencies> + + <!-- Test with JUnit5 --> + <dependency> + <groupId>org.junit.jupiter</groupId> + <artifactId>junit-jupiter-api</artifactId> + <version>5.4.2</version> + <scope>test</scope> + </dependency> + <dependency> + <groupId>org.junit.jupiter</groupId> + <artifactId>junit-jupiter-engine</artifactId> + <version>5.4.2</version> + <scope>test</scope> + </dependency> + </dependencies> + + <build> + <plugins> + <!-- Compiling code --> + <plugin> + <artifactId>maven-compiler-plugin</artifactId> + <version>3.8.1</version> + <configuration> + <encoding>UTF-8</encoding> + <release>13</release> + </configuration> + </plugin> + <plugin> + <groupId>org.apache.maven.plugins</groupId> + <artifactId>maven-surefire-plugin</artifactId> + <version>3.0.0-M5</version> + <!-- + <configuration> + <argLine>${argLine} dash dash add-exports javafx.graphics/com.sun.javafx.application=ALL-UNNAMED</argLine> + </configuration> + --> + </plugin> + </plugins> + </build> +</project> \ No newline at end of file diff --git a/antipatterns/src/main/java/antipatterns/DataClassWithEffectivelyFinalFields.java b/antipatterns/src/main/java/antipatterns/DataClassWithEffectivelyFinalFields.java new file mode 100644 index 0000000..f5b4597 --- /dev/null +++ b/antipatterns/src/main/java/antipatterns/DataClassWithEffectivelyFinalFields.java @@ -0,0 +1,20 @@ +package antipatterns; + +public class DataClassWithEffectivelyFinalFields { + + private final String name; // should be final + private final int number; // should be final + + public DataClassWithEffectivelyFinalFields(final String name, final int number) { + this.name = name; + this.number = number; + } + + public String getName() { + return name; + } + + public int getNumber() { + return number; + } +} diff --git a/antipatterns/src/main/java/antipatterns/DataClassWithLittleLogic.java b/antipatterns/src/main/java/antipatterns/DataClassWithLittleLogic.java new file mode 100644 index 0000000..5549e27 --- /dev/null +++ b/antipatterns/src/main/java/antipatterns/DataClassWithLittleLogic.java @@ -0,0 +1,35 @@ +package antipatterns; + +// ok, but see the test +public class DataClassWithLittleLogic { + + private String name; + private int number; + + public DataClassWithLittleLogic(final String name, final int number) { + this.name = name; + this.number = number; + } + + @Override + public String toString() { + return String.format("[DataClassWithLittleLogic name=%s number=%s]", name, number); + } + + public DataClassWithLittleLogic() { + } + + public String getName() { + return name; + } + public void setName(final String name) { + this.name = name; + } + + public int getNumber() { + return number; + } + public void setNumber(final int number) { + this.number = number; + } +} diff --git a/antipatterns/src/main/java/antipatterns/DataClassWithValidation.java b/antipatterns/src/main/java/antipatterns/DataClassWithValidation.java new file mode 100644 index 0000000..2569b6c --- /dev/null +++ b/antipatterns/src/main/java/antipatterns/DataClassWithValidation.java @@ -0,0 +1,26 @@ +package antipatterns; + +// see DataClassWithValidationTest +public class DataClassWithValidation { + + private String name; + + public DataClassWithValidation(final String name) { + // should use setName to ensure validation logic is run + this.name = name; + } + + public DataClassWithValidation() { + } + + public String getName() { + return name; + } + public void setName(final String name) { + if (name.startsWith(" ")) { + // should rather use trim() or strip() and check the length + throw new IllegalArgumentException("Name cannot begin with blank"); + } + this.name = name; + } +} diff --git a/antipatterns/src/main/java/antipatterns/NonEncapsulatedList.java b/antipatterns/src/main/java/antipatterns/NonEncapsulatedList.java new file mode 100644 index 0000000..d8882d2 --- /dev/null +++ b/antipatterns/src/main/java/antipatterns/NonEncapsulatedList.java @@ -0,0 +1,21 @@ +package antipatterns; + +import java.util.Collection; +import java.util.Iterator; + +public class NonEncapsulatedList implements Iterable<String> { + + private final Collection<String> names; + + public NonEncapsulatedList(final Collection<String> names) { + this.names = names; + } + + public Collection<String> getNames() { + return names; + } + + public Iterator<String> iterator() { + return names.iterator(); + } +} diff --git a/antipatterns/src/test/java/antipatterns/DataClassWithLittleLogicTest.java b/antipatterns/src/test/java/antipatterns/DataClassWithLittleLogicTest.java new file mode 100644 index 0000000..3090452 --- /dev/null +++ b/antipatterns/src/test/java/antipatterns/DataClassWithLittleLogicTest.java @@ -0,0 +1,32 @@ +package antipatterns; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class DataClassWithLittleLogicTest { + + private DataClassWithLittleLogic instance1, instance2; + + @BeforeEach + public void setup() { + instance1 = new DataClassWithLittleLogic(); + instance2 = new DataClassWithLittleLogic("High", 5); + } + + @Test + public void testConstructor() { + Assertions.assertEquals("High", instance2.getName()); + } + + @Test + public void testToString() { + Assertions.assertEquals("[DataClassWithLittleLogic name=High number=5]", instance2.toString()); + } + + @Test + public void testSetName() { + instance1.setName("Low"); + Assertions.assertEquals("Low", instance1.getName()); + } +} diff --git a/antipatterns/src/test/java/antipatterns/DataClassWithValidationTest.java b/antipatterns/src/test/java/antipatterns/DataClassWithValidationTest.java new file mode 100644 index 0000000..a497e5a --- /dev/null +++ b/antipatterns/src/test/java/antipatterns/DataClassWithValidationTest.java @@ -0,0 +1,34 @@ +package antipatterns; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class DataClassWithValidationTest { + + @Test + public void testSetName() { + try { + new DataClassWithValidation(" name starting with blank"); + Assertions.fail("Should throw IllegalArgumentException"); + } catch (final IllegalArgumentException iae) { + } catch (final Exception iae) { + Assertions.fail("Should throw IllegalArgumentException"); + } + + try { + new DataClassWithValidation("ok name"); + } catch (final Exception iae) { + Assertions.fail(); + } + } + + @Test + public void testBetterSetName_invalidName() { + Assertions.assertThrows(IllegalArgumentException.class, () -> new DataClassWithValidation(" name starting with blank")); + } + + @Test + public void testBetterSetName_validName() { + new DataClassWithValidation("ok name"); + } +} -- GitLab