From 480b51e830ad9f64a363b86bbf183879a5a76728 Mon Sep 17 00:00:00 2001 From: Evgeny Mandrikov Date: Fri, 3 Feb 2012 16:14:58 +0400 Subject: [PATCH] Fix some quality flaws --- .../sonar/plugins/cpd/SonarEngineTest.java | 20 +++--- .../net/sourceforge/pmd/cpd/Language.java | 2 - .../OriginalCloneDetectionAlgorithm.java | 2 +- .../suffixtree/DuplicationsCollector.java | 12 ++-- .../sonar/duplications/index/CloneGroup.java | 61 ++++++++++++++----- .../detector/original/FilterTest.java | 16 +++-- 6 files changed, 69 insertions(+), 44 deletions(-) diff --git a/plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/SonarEngineTest.java b/plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/SonarEngineTest.java index 2b6c2b39a4e..434049fdbd6 100644 --- a/plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/SonarEngineTest.java +++ b/plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/SonarEngineTest.java @@ -19,16 +19,6 @@ */ package org.sonar.plugins.cpd; -import static org.mockito.Matchers.argThat; -import static org.mockito.Matchers.eq; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; - -import java.util.Arrays; -import java.util.Collections; -import java.util.List; - import org.junit.Before; import org.junit.Test; import org.sonar.api.batch.SensorContext; @@ -39,6 +29,14 @@ import org.sonar.api.test.IsMeasure; import org.sonar.duplications.index.CloneGroup; import org.sonar.duplications.index.ClonePart; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.*; + public class SonarEngineTest { private SensorContext context; @@ -132,7 +130,7 @@ public class SonarEngineTest { } private CloneGroup newCloneGroup(ClonePart... parts) { - return new CloneGroup(0, parts[0], Arrays.asList(parts)); + return CloneGroup.builder().setLength(0).setOrigin(parts[0]).setParts(Arrays.asList(parts)).build(); } } diff --git a/sonar-duplications/src/main/java/net/sourceforge/pmd/cpd/Language.java b/sonar-duplications/src/main/java/net/sourceforge/pmd/cpd/Language.java index 6b1e11c643c..8fb352cfe84 100644 --- a/sonar-duplications/src/main/java/net/sourceforge/pmd/cpd/Language.java +++ b/sonar-duplications/src/main/java/net/sourceforge/pmd/cpd/Language.java @@ -27,8 +27,6 @@ import java.io.FilenameFilter; public interface Language { - String fileSeparator = System.getProperty("file.separator"); - Tokenizer getTokenizer(); FilenameFilter getFileFilter(); diff --git a/sonar-duplications/src/main/java/org/sonar/duplications/detector/original/OriginalCloneDetectionAlgorithm.java b/sonar-duplications/src/main/java/org/sonar/duplications/detector/original/OriginalCloneDetectionAlgorithm.java index bef9f4e80a4..afefdef5c07 100644 --- a/sonar-duplications/src/main/java/org/sonar/duplications/detector/original/OriginalCloneDetectionAlgorithm.java +++ b/sonar-duplications/src/main/java/org/sonar/duplications/detector/original/OriginalCloneDetectionAlgorithm.java @@ -223,7 +223,7 @@ public final class OriginalCloneDetectionAlgorithm { parts.add(part); } - filter.add(new CloneGroup(cloneLength, origin, parts)); + filter.add(CloneGroup.builder().setLength(cloneLength).setOrigin(origin).setParts(parts).build()); } } diff --git a/sonar-duplications/src/main/java/org/sonar/duplications/detector/suffixtree/DuplicationsCollector.java b/sonar-duplications/src/main/java/org/sonar/duplications/detector/suffixtree/DuplicationsCollector.java index 6c05737240b..b36f5b3c94f 100644 --- a/sonar-duplications/src/main/java/org/sonar/duplications/detector/suffixtree/DuplicationsCollector.java +++ b/sonar-duplications/src/main/java/org/sonar/duplications/detector/suffixtree/DuplicationsCollector.java @@ -80,8 +80,10 @@ public class DuplicationsCollector extends Search.Collector { */ @Override public void endOfGroup() { - int lengthInUnits = 0; ClonePart origin = null; + + CloneGroup.Builder builder = CloneGroup.builder().setLength(length); + List parts = Lists.newArrayListWithCapacity(count); for (int[] b : blockNumbers) { Block firstBlock = text.getBlock(b[0]); @@ -97,7 +99,7 @@ public class DuplicationsCollector extends Search.Collector { if (origin == null) { origin = part; // To calculate length important to use the origin, because otherwise block may come from DB without required data - lengthInUnits = lastBlock.getEndUnit() - firstBlock.getStartUnit() + 1; + builder.setLengthInUnits(lastBlock.getEndUnit() - firstBlock.getStartUnit() + 1); } else if (part.getUnitStart() < origin.getUnitStart()) { origin = part; } @@ -107,11 +109,9 @@ public class DuplicationsCollector extends Search.Collector { } Collections.sort(parts, ContainsInComparator.CLONEPART_COMPARATOR); + builder.setOrigin(origin).setParts(parts); - CloneGroup group = new CloneGroup(length, origin, parts); - group.setLengthInUnits(lengthInUnits); - - filter(group); + filter(builder.build()); reset(); } diff --git a/sonar-duplications/src/main/java/org/sonar/duplications/index/CloneGroup.java b/sonar-duplications/src/main/java/org/sonar/duplications/index/CloneGroup.java index dfb86bb93d6..eda0b40b8cd 100644 --- a/sonar-duplications/src/main/java/org/sonar/duplications/index/CloneGroup.java +++ b/sonar-duplications/src/main/java/org/sonar/duplications/index/CloneGroup.java @@ -38,10 +38,52 @@ public class CloneGroup { */ private int hash; - public CloneGroup(int cloneLength, ClonePart origin, List parts) { - this.cloneLength = cloneLength; - this.originPart = origin; - this.parts = ImmutableList.copyOf(parts); + /** + * @since 2.14 + */ + public static Builder builder() { + return new Builder(); + } + + /** + * @since 2.14 + */ + public static final class Builder { + private ClonePart origin; + private int length; + private int lengthInUnits; + private List parts; + + public Builder setLength(int length) { + this.length = length; + return this; + } + + public Builder setOrigin(ClonePart origin) { + this.origin = origin; + return this; + } + + public Builder setParts(List parts) { + this.parts = ImmutableList.copyOf(parts); + return this; + } + + public Builder setLengthInUnits(int length) { + this.lengthInUnits = length; + return this; + } + + public CloneGroup build() { + return new CloneGroup(this); + } + } + + private CloneGroup(Builder builder) { + this.cloneLength = builder.length; + this.originPart = builder.origin; + this.parts = builder.parts; + this.length = builder.lengthInUnits; } public ClonePart getOriginPart() { @@ -60,17 +102,6 @@ public class CloneGroup { return length; } - /** - * TODO get rid of this method, otherwise class is not immutable - * - * @since 2.14 - * @see #getLengthInUnits() - */ - @Beta - public void setLengthInUnits(int length) { - this.length = length; - } - /** * @return clone length in {@link org.sonar.duplications.block.Block}s */ diff --git a/sonar-duplications/src/test/java/org/sonar/duplications/detector/original/FilterTest.java b/sonar-duplications/src/test/java/org/sonar/duplications/detector/original/FilterTest.java index bf60290a6ab..6ed2a9e1a41 100644 --- a/sonar-duplications/src/test/java/org/sonar/duplications/detector/original/FilterTest.java +++ b/sonar-duplications/src/test/java/org/sonar/duplications/detector/original/FilterTest.java @@ -19,18 +19,16 @@ */ package org.sonar.duplications.detector.original; -import static org.hamcrest.Matchers.is; -import static org.junit.Assert.assertThat; -import static org.mockito.Mockito.spy; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; - -import java.util.Arrays; - import org.junit.Test; import org.sonar.duplications.index.CloneGroup; import org.sonar.duplications.index.ClonePart; +import java.util.Arrays; + +import static org.hamcrest.Matchers.is; +import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.*; + public class FilterTest { /** @@ -187,7 +185,7 @@ public class FilterTest { * Creates new group from list of parts, origin - is a first part from list. */ private CloneGroup newCloneGroup(int len, ClonePart... parts) { - return new CloneGroup(len, parts[0], Arrays.asList(parts)); + return CloneGroup.builder().setLength(len).setOrigin(parts[0]).setParts(Arrays.asList(parts)).build(); } } -- 2.39.5