From ee6a0b5ba26a032b6ad7808d42aa46f23e276ad1 Mon Sep 17 00:00:00 2001 From: Erlend Kristiansen Date: Mon, 9 Dec 2013 21:20:22 +0100 Subject: [PATCH] Adding the interface PropertiesExtractor. Flattening out the extractor class hierarchy. --- .../LongestCommonPrefixExtractorTest.java | 10 ++-- .../changers/RefaktorChangerException.java | 2 +- .../extractors/AbstractPropertyExtractor.java | 59 ------------------- .../refaktor/extractors/CollectorManager.java | 43 ++++++++++++++ ...ExtractAndMoveMethodPrefixesExtractor.java | 27 +++++---- .../LongestCommonPrefixExtractor.java | 28 ++++----- .../extractors/PrefixViewExtractor.java | 3 +- .../extractors/PropertiesExtractor.java | 7 +++ .../extractors/PropertyExtractor.java | 8 --- .../LongestCommonPrefixCollector.java | 5 ++ .../collectors/PrefixesCollector.java | 5 ++ .../collectors/PropertyCollector.java | 2 + .../collectors/UnfixesCollector.java | 5 ++ .../no/uio/ifi/refaktor/prefix/PrefixSet.java | 4 ++ .../no/uio/ifi/refaktor/views/PrefixView.java | 11 ++-- 15 files changed, 113 insertions(+), 106 deletions(-) delete mode 100644 software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/AbstractPropertyExtractor.java create mode 100644 software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/CollectorManager.java create mode 100644 software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PropertiesExtractor.java delete mode 100644 software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PropertyExtractor.java diff --git a/software/no.uio.ifi.refaktor.tests/src/no/uio/ifi/refaktor/tests/LongestCommonPrefixExtractorTest.java b/software/no.uio.ifi.refaktor.tests/src/no/uio/ifi/refaktor/tests/LongestCommonPrefixExtractorTest.java index 6966495a..9e55e181 100644 --- a/software/no.uio.ifi.refaktor.tests/src/no/uio/ifi/refaktor/tests/LongestCommonPrefixExtractorTest.java +++ b/software/no.uio.ifi.refaktor.tests/src/no/uio/ifi/refaktor/tests/LongestCommonPrefixExtractorTest.java @@ -33,7 +33,7 @@ public class LongestCommonPrefixExtractorTest { @Test public void testExtractPropertyStartFoo() { LongestCommonPrefixExtractor extractor = makeStartFooExtractor(); - extractor.extractProperty(); + extractor.extractProperties(); assertEquals("a.b.c", extractor.stringProperty()); } @@ -59,7 +59,7 @@ public class LongestCommonPrefixExtractorTest { @Test public void testExtractPropertySingleStatement() { LongestCommonPrefixExtractor extractor = makeSingleStatementExtractor(); - extractor.extractProperty(); + extractor.extractProperties(); assertEquals("a.b.c", extractor.stringProperty()); } @@ -85,7 +85,7 @@ public class LongestCommonPrefixExtractorTest { @Test public void testExtractPropertyMethodText() { LongestCommonPrefixExtractor extractor = makeMethodTextExtractor(); - extractor.extractProperty(); + extractor.extractProperties(); assertEquals("a.b", extractor.stringProperty()); } @@ -104,7 +104,7 @@ public class LongestCommonPrefixExtractorTest { @Test public void textExtractPropertyBlockText() { LongestCommonPrefixExtractor extractor = makeBlockTextExtractor(); - extractor.extractProperty(); + extractor.extractProperties(); assertEquals("a.b", extractor.stringProperty()); } @@ -120,7 +120,7 @@ public class LongestCommonPrefixExtractorTest { CompilationUnitTextSelection selection = new CompilationUnitTextSelection(icu, 218, 62); LongestCommonPrefixExtractor extractor = new LongestCommonPrefixExtractor(selection); assertTrue(extractor.selectionIsValid()); - extractor.extractProperty(); + extractor.extractProperties(); assertEquals("a.c", extractor.stringProperty()); } diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/changers/RefaktorChangerException.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/changers/RefaktorChangerException.java index db86f8a9..fda2dc86 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/changers/RefaktorChangerException.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/changers/RefaktorChangerException.java @@ -5,7 +5,7 @@ public class RefaktorChangerException extends RuntimeException { private static final long serialVersionUID = 7037529198855202086L; public enum Reason { - SELECTION_INVALID("The selected text is not valid for this refactoring."), + SELECTION_INVALID("The selected text is not valid."), NO_TARGET_FOUND("Could not find target for move."); private final String message; diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/AbstractPropertyExtractor.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/AbstractPropertyExtractor.java deleted file mode 100644 index c0e27363..00000000 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/AbstractPropertyExtractor.java +++ /dev/null @@ -1,59 +0,0 @@ -package no.uio.ifi.refaktor.extractors; - -import java.util.LinkedList; -import java.util.List; - -import no.uio.ifi.refaktor.extractors.collectors.PropertyCollector; -import no.uio.ifi.refaktor.extractors.collectors.SelectionChecker; -import no.uio.ifi.refaktor.utils.CompilationUnitTextSelection; - -import org.eclipse.jdt.core.dom.ASTNode; -import org.eclipse.jdt.core.dom.ASTVisitor; - -/** - * A hierarchy of classes that are used - * to extract properties from source code. - */ -public abstract class AbstractPropertyExtractor implements PropertyExtractor { - - private final CompilationUnitTextSelection selection; - protected boolean isExtracted = false; - - public AbstractPropertyExtractor(CompilationUnitTextSelection selection) { - this.selection = selection; - } - - @Override - public void extractProperty() { - assert !isExtracted : "should only be called once" ; - if (!selectionIsValid()) - throw new RuntimeException(this.getClass().getCanonicalName() + ": Trying to extract property from invalid selection!"); - for (ASTVisitor collector: getCollectors()) - getStartNode().accept(collector); - isExtracted = true; - } - - private List getCollectors() { - List visitors = new LinkedList(); - registerCollectors(visitors); - return visitors; - } - - /** - * Every extractor must register its collectors by adding them to the given list. - * See {@link List#add(Object)}. - * - * @param collectors The list that each collector should be added to. - */ - protected abstract void registerCollectors(List collectors); - - private ASTNode getStartNode() { - return selection.getCoveringNode(); - } - - @Override - public boolean selectionIsValid() { - return SelectionChecker.isSelectionValid(selection); - } - -} diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/CollectorManager.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/CollectorManager.java new file mode 100644 index 00000000..c1673c0a --- /dev/null +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/CollectorManager.java @@ -0,0 +1,43 @@ +package no.uio.ifi.refaktor.extractors; + +import java.util.Arrays; +import java.util.LinkedList; + +import no.uio.ifi.refaktor.extractors.collectors.PropertyCollector; +import no.uio.ifi.refaktor.utils.CompilationUnitTextSelection; + +import org.eclipse.jdt.core.dom.ASTNode; + +public class CollectorManager { + + private final CompilationUnitTextSelection selection; + private final LinkedList collectors; + + public CollectorManager(CompilationUnitTextSelection selection, PropertyCollector ... collectors) { + this.selection = selection; + this.collectors = new LinkedList(Arrays.asList(collectors)); + } + + public static void collectProperties(CompilationUnitTextSelection selection, PropertyCollector ... collectors) { + new CollectorManager(selection, collectors).collectProperties(); + } + + private void collectProperties() { + clearCollectorsData(); + runCollectors(); + } + + private void clearCollectorsData() { + for (PropertyCollector collector: collectors) + collector.clearData(); + } + + private void runCollectors() { + for (PropertyCollector collector: collectors) + getStartNode().accept(collector); + } + + private ASTNode getStartNode() { + return selection.getCoveringNode(); + } +} diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/ExtractAndMoveMethodPrefixesExtractor.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/ExtractAndMoveMethodPrefixesExtractor.java index 64560e64..e0380e73 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/ExtractAndMoveMethodPrefixesExtractor.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/ExtractAndMoveMethodPrefixesExtractor.java @@ -3,13 +3,12 @@ package no.uio.ifi.refaktor.extractors; import java.util.Collections; import java.util.Comparator; import java.util.LinkedList; -import java.util.List; import no.uio.ifi.refaktor.changers.ExtractAndMoveMethodChanger; import no.uio.ifi.refaktor.changers.RefaktorChangerException; import no.uio.ifi.refaktor.changers.RefaktorChangerException.Reason; import no.uio.ifi.refaktor.extractors.collectors.PrefixesCollector; -import no.uio.ifi.refaktor.extractors.collectors.PropertyCollector; +import no.uio.ifi.refaktor.extractors.collectors.SelectionChecker; import no.uio.ifi.refaktor.extractors.collectors.UnfixesCollector; import no.uio.ifi.refaktor.prefix.Prefix; import no.uio.ifi.refaktor.prefix.PrefixSet; @@ -27,24 +26,19 @@ import no.uio.ifi.refaktor.utils.CompilationUnitTextSelection; * * The class is typically used by the {@link ExtractAndMoveMethodChanger}. */ -public class ExtractAndMoveMethodPrefixesExtractor extends AbstractPropertyExtractor { +public class ExtractAndMoveMethodPrefixesExtractor implements PropertiesExtractor { + private final CompilationUnitTextSelection selection; private final PrefixesCollector prefixesCollector; private final UnfixesCollector unfixesCollector; private PrefixSet safePrefixes = null; public ExtractAndMoveMethodPrefixesExtractor(CompilationUnitTextSelection selection) { - super(selection); + this.selection = selection; prefixesCollector = new PrefixesCollector(selection); unfixesCollector = new UnfixesCollector(selection); } - @Override - protected void registerCollectors(List collectors) { - collectors.add(prefixesCollector); - collectors.add(unfixesCollector); - } - public PrefixSet getSafePrefixes() { if (safePrefixes == null) { safePrefixes = createSafePrefixes(); @@ -98,7 +92,7 @@ public class ExtractAndMoveMethodPrefixesExtractor extends AbstractPropertyExtra public void checkPreconditions() throws RefaktorChangerException { checkIfSelectionIsValid(); - extractProperty(); + extractProperties(); checkIfUsefulTargetFound(); } @@ -107,8 +101,17 @@ public class ExtractAndMoveMethodPrefixesExtractor extends AbstractPropertyExtra throw new RefaktorChangerException(Reason.SELECTION_INVALID); } - public void checkIfUsefulTargetFound() { + private void checkIfUsefulTargetFound() { if (!hasUsefulResults()) throw new RefaktorChangerException(Reason.NO_TARGET_FOUND); } + + @Override + public void extractProperties() { + CollectorManager.collectProperties(selection, prefixesCollector, unfixesCollector); + } + + private boolean selectionIsValid() { + return SelectionChecker.isSelectionValid(selection); + } } diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/LongestCommonPrefixExtractor.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/LongestCommonPrefixExtractor.java index 5c58eadb..e6881b1f 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/LongestCommonPrefixExtractor.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/LongestCommonPrefixExtractor.java @@ -1,31 +1,29 @@ package no.uio.ifi.refaktor.extractors; -import java.util.List; - import no.uio.ifi.refaktor.extractors.collectors.LongestCommonPrefixCollector; -import no.uio.ifi.refaktor.extractors.collectors.PropertyCollector; -import no.uio.ifi.refaktor.prefix.Prefix; +import no.uio.ifi.refaktor.extractors.collectors.SelectionChecker; import no.uio.ifi.refaktor.utils.CompilationUnitTextSelection; -public class LongestCommonPrefixExtractor extends AbstractPropertyExtractor { +public class LongestCommonPrefixExtractor implements PropertiesExtractor { - private LongestCommonPrefixCollector longestCommonPrefixCollector; + private final CompilationUnitTextSelection selection; + private final LongestCommonPrefixCollector longestCommonPrefixCollector; public LongestCommonPrefixExtractor(CompilationUnitTextSelection selection) { - super(selection); + this.selection = selection; longestCommonPrefixCollector = new LongestCommonPrefixCollector(selection); } - @Override - protected void registerCollectors(List collectors) { - collectors.add(longestCommonPrefixCollector); + public String stringProperty() { + return longestCommonPrefixCollector.getProperty().toString(); } - - public Prefix getProperty() { - return longestCommonPrefixCollector.getProperty(); + + @Override + public void extractProperties() { + CollectorManager.collectProperties(selection, longestCommonPrefixCollector); } - public String stringProperty() { - return longestCommonPrefixCollector.getProperty().toString(); + public boolean selectionIsValid() { + return SelectionChecker.isSelectionValid(selection); } } \ No newline at end of file diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PrefixViewExtractor.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PrefixViewExtractor.java index 35731a00..5303b5b3 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PrefixViewExtractor.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PrefixViewExtractor.java @@ -10,8 +10,7 @@ public class PrefixViewExtractor extends ExtractAndMoveMethodPrefixesExtractor { @Override public String toString() { - assert selectionIsValid(); - assert isExtracted; + checkIfSelectionIsValid(); String str = "Prefixes:\n" + getPrefixes().toString(); str += "\nUnfixes:\n" + getUnfixes().toString(); str += "\nSafe Prefixes:\n" + getSafePrefixes().toString(); diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PropertiesExtractor.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PropertiesExtractor.java new file mode 100644 index 00000000..18534cb5 --- /dev/null +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PropertiesExtractor.java @@ -0,0 +1,7 @@ +package no.uio.ifi.refaktor.extractors; + +public interface PropertiesExtractor { + + public void extractProperties(); + +} \ No newline at end of file diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PropertyExtractor.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PropertyExtractor.java deleted file mode 100644 index 7cc7d5af..00000000 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/PropertyExtractor.java +++ /dev/null @@ -1,8 +0,0 @@ -package no.uio.ifi.refaktor.extractors; - -public interface PropertyExtractor { - - public void extractProperty(); - public boolean selectionIsValid(); - -} \ No newline at end of file diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/LongestCommonPrefixCollector.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/LongestCommonPrefixCollector.java index 56663582..3cf612bc 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/LongestCommonPrefixCollector.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/LongestCommonPrefixCollector.java @@ -21,4 +21,9 @@ public class LongestCommonPrefixCollector extends AbstractPrefixCollector { public Prefix getProperty() { return longestCommonPrefix; } + + @Override + public void clearData() { + longestCommonPrefix = null; + } } \ No newline at end of file diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/PrefixesCollector.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/PrefixesCollector.java index 2bd4f2c1..82a3b6d1 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/PrefixesCollector.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/PrefixesCollector.java @@ -20,4 +20,9 @@ public class PrefixesCollector extends AbstractPrefixCollector { public PrefixSet getPrefixes() { return prefixes; } + + @Override + public void clearData() { + prefixes.clear(); + } } \ No newline at end of file diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/PropertyCollector.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/PropertyCollector.java index 445462a0..76241c06 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/PropertyCollector.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/PropertyCollector.java @@ -34,4 +34,6 @@ public abstract class PropertyCollector extends ASTVisitor { protected boolean nodeInSelection(ASTNode node) { return selection.surroundsNode(node); } + + public abstract void clearData(); } \ No newline at end of file diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/UnfixesCollector.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/UnfixesCollector.java index 6254c8ab..1edaf6d9 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/UnfixesCollector.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/extractors/collectors/UnfixesCollector.java @@ -55,4 +55,9 @@ public class UnfixesCollector extends PropertyCollector { RefaktorDebug.println("Unfix string: " + p.toString()); unfixes.add(p); } + + @Override + public void clearData() { + unfixes.clear(); + } } \ No newline at end of file diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/prefix/PrefixSet.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/prefix/PrefixSet.java index 0fe9674c..0d2c1da1 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/prefix/PrefixSet.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/prefix/PrefixSet.java @@ -113,4 +113,8 @@ public class PrefixSet implements Iterable { } } } + + public void clear() { + prefixes.clear(); + } } diff --git a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/views/PrefixView.java b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/views/PrefixView.java index de7f9141..c1b36a1b 100644 --- a/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/views/PrefixView.java +++ b/software/no.uio.ifi.refaktor/src/no/uio/ifi/refaktor/views/PrefixView.java @@ -1,6 +1,7 @@ package no.uio.ifi.refaktor.views; +import no.uio.ifi.refaktor.changers.RefaktorChangerException; import no.uio.ifi.refaktor.extractors.PrefixViewExtractor; import no.uio.ifi.refaktor.utils.CompilationUnitTextSelection; import no.uio.ifi.refaktor.utils.DocumentUtils; @@ -120,12 +121,14 @@ public class PrefixView extends ViewPart implements ISelectionChangedListener, I ICompilationUnit compilationUnit = RefaktorHandleUtils.getCompilationUnitFromEditor(editor); CompilationUnitTextSelection compilationUnitTextSelection = new CompilationUnitTextSelection(compilationUnit, strippedTextSelection); final PrefixViewExtractor extractor = new PrefixViewExtractor(compilationUnitTextSelection); - if (extractor.selectionIsValid()) { - extractor.extractProperty(); + try { + extractor.checkIfSelectionIsValid(); + extractor.extractProperties(); String dialogText = extractor.toString(); doc.set(dialogText); - } else - doc.set("invalid selection"); + } catch (RefaktorChangerException e) { + doc.set(e.getMessage()); + } } } \ No newline at end of file -- 2.43.5