From dd242aa735b1efa3cf7664a3b64afd32bb5b28dd Mon Sep 17 00:00:00 2001
From: Assaf <asafyi@gmail.com>
Date: Wed, 20 Jun 2018 16:14:05 +0300
Subject: [PATCH] Remove code duplication - extract Labels and debugItems from
 MethodLocation.

---
 .../jf/dexlib2/builder/BuilderDebugItem.java  |   6 +-
 .../jf/dexlib2/builder/ItemWithLocation.java  |  16 ++
 .../java/org/jf/dexlib2/builder/Label.java    |   9 +-
 .../jf/dexlib2/builder/LocatedDebugItems.java |  10 ++
 .../org/jf/dexlib2/builder/LocatedItems.java  |  87 +++++++++
 .../org/jf/dexlib2/builder/LocatedLabels.java |   9 +
 .../jf/dexlib2/builder/MethodLocation.java    | 166 ++----------------
 7 files changed, 135 insertions(+), 168 deletions(-)
 create mode 100644 dexlib2/src/main/java/org/jf/dexlib2/builder/ItemWithLocation.java
 create mode 100644 dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedDebugItems.java
 create mode 100644 dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedItems.java
 create mode 100644 dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedLabels.java

diff --git a/dexlib2/src/main/java/org/jf/dexlib2/builder/BuilderDebugItem.java b/dexlib2/src/main/java/org/jf/dexlib2/builder/BuilderDebugItem.java
index 0969a9db..41effa1f 100644
--- a/dexlib2/src/main/java/org/jf/dexlib2/builder/BuilderDebugItem.java
+++ b/dexlib2/src/main/java/org/jf/dexlib2/builder/BuilderDebugItem.java
@@ -33,10 +33,7 @@ package org.jf.dexlib2.builder;
 
 import org.jf.dexlib2.iface.debug.DebugItem;
 
-import javax.annotation.Nullable;
-
-public abstract class BuilderDebugItem implements DebugItem {
-    @Nullable MethodLocation location;
+public abstract class BuilderDebugItem extends ItemWithLocation implements DebugItem {
 
     public BuilderDebugItem() {
     }
@@ -48,5 +45,4 @@ public abstract class BuilderDebugItem implements DebugItem {
         }
         return location.getCodeAddress();
     }
-
 }
diff --git a/dexlib2/src/main/java/org/jf/dexlib2/builder/ItemWithLocation.java b/dexlib2/src/main/java/org/jf/dexlib2/builder/ItemWithLocation.java
new file mode 100644
index 00000000..f6515528
--- /dev/null
+++ b/dexlib2/src/main/java/org/jf/dexlib2/builder/ItemWithLocation.java
@@ -0,0 +1,16 @@
+package org.jf.dexlib2.builder;
+
+import javax.annotation.Nullable;
+
+public abstract class ItemWithLocation {
+    @Nullable
+    MethodLocation location;
+
+    public boolean isPlaced() {
+        return location != null;
+    }
+
+    public void setLocation(MethodLocation methodLocation) {
+        location = methodLocation;
+    }
+}
diff --git a/dexlib2/src/main/java/org/jf/dexlib2/builder/Label.java b/dexlib2/src/main/java/org/jf/dexlib2/builder/Label.java
index ef19f91c..35c3f562 100644
--- a/dexlib2/src/main/java/org/jf/dexlib2/builder/Label.java
+++ b/dexlib2/src/main/java/org/jf/dexlib2/builder/Label.java
@@ -32,11 +32,8 @@
 package org.jf.dexlib2.builder;
 
 import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-
-public class Label {
-    @Nullable MethodLocation location;
 
+public class Label extends ItemWithLocation {
     Label() {
     }
 
@@ -55,8 +52,4 @@ public class Label {
         }
         return location;
     }
-
-    public boolean isPlaced() {
-        return location != null;
-    }
 }
diff --git a/dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedDebugItems.java b/dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedDebugItems.java
new file mode 100644
index 00000000..61403565
--- /dev/null
+++ b/dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedDebugItems.java
@@ -0,0 +1,10 @@
+package org.jf.dexlib2.builder;
+
+public class LocatedDebugItems extends LocatedItems<BuilderDebugItem> {
+
+    @Override
+    protected String addLocatedItemError() {
+        return "Cannot add a debug item that has already been added to a method." +
+                "You must remove it from its current location first.";
+    }
+}
diff --git a/dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedItems.java b/dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedItems.java
new file mode 100644
index 00000000..c4abf9e3
--- /dev/null
+++ b/dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedItems.java
@@ -0,0 +1,87 @@
+package org.jf.dexlib2.builder;
+
+import com.google.common.collect.ImmutableList;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import java.util.*;
+
+public abstract class LocatedItems<T extends ItemWithLocation> {
+    // We end up creating and keeping around a *lot* of MethodLocation objects
+    // when building a new dex file, so it's worth the trouble of lazily creating
+    // the labels and debugItems lists only when they are needed
+    @Nullable
+    private List<T> items = null;
+
+    @Nonnull
+    private List<T> getMutableItems() {
+        if (items == null) {
+            items = new ArrayList<>(1);
+        }
+        return items;
+    }
+
+    @Nonnull
+    private List<T> getImmutableItems() {
+        if (items == null) {
+            return ImmutableList.of();
+        }
+        return items;
+    }
+
+    public Set<T> getModifiableItems(MethodLocation newItemsLocation) {
+        return new AbstractSet<T>() {
+            @Nonnull
+            @Override public Iterator<T> iterator() {
+                final Iterator<T> it = getImmutableItems().iterator();
+
+                return new Iterator<T>() {
+                    private @Nullable
+                    T currentItem = null;
+
+                    @Override public boolean hasNext() {
+                        return it.hasNext();
+                    }
+
+                    @Override public T next() {
+                        currentItem = it.next();
+                        return currentItem;
+                    }
+
+                    @Override public void remove() {
+                        if (currentItem != null) {
+                            currentItem.setLocation(null);
+                        }
+                        it.remove();
+                    }
+                };
+            }
+
+            @Override public int size() {
+                return getImmutableItems().size();
+            }
+
+            @Override public boolean add(@Nonnull T item) {
+                if (item.isPlaced()) {
+                    throw new IllegalArgumentException(addLocatedItemError());
+                }
+                item.setLocation(newItemsLocation);
+                getMutableItems().add(item);
+                return true;
+            }
+        };
+    }
+
+    protected abstract String addLocatedItemError();
+
+    public void mergeItemsInto(@Nonnull MethodLocation newLocation, LocatedItems<T> otherLocatedItems) {
+        if (items != null || otherLocatedItems.items != null) {
+            List<T> otherItems = otherLocatedItems.getMutableItems();
+            for (T item: getImmutableItems()) {
+                item.setLocation(newLocation);
+                otherItems.add(item);
+            }
+            items = null;
+        }
+    }
+}
diff --git a/dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedLabels.java b/dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedLabels.java
new file mode 100644
index 00000000..2b7823f3
--- /dev/null
+++ b/dexlib2/src/main/java/org/jf/dexlib2/builder/LocatedLabels.java
@@ -0,0 +1,9 @@
+package org.jf.dexlib2.builder;
+
+public class LocatedLabels extends LocatedItems<Label> {
+    @Override
+    protected String addLocatedItemError() {
+        return "Cannot add a label that is already placed." +
+                "You must remove it from its current location first.";
+    }
+}
diff --git a/dexlib2/src/main/java/org/jf/dexlib2/builder/MethodLocation.java b/dexlib2/src/main/java/org/jf/dexlib2/builder/MethodLocation.java
index 762aadae..ea1fb7ee 100644
--- a/dexlib2/src/main/java/org/jf/dexlib2/builder/MethodLocation.java
+++ b/dexlib2/src/main/java/org/jf/dexlib2/builder/MethodLocation.java
@@ -31,7 +31,6 @@
 
 package org.jf.dexlib2.builder;
 
-import com.google.common.collect.ImmutableList;
 import org.jf.dexlib2.builder.debug.*;
 import org.jf.dexlib2.iface.instruction.Instruction;
 import org.jf.dexlib2.iface.reference.StringReference;
@@ -46,16 +45,13 @@ public class MethodLocation {
     int codeAddress;
     int index;
 
-    // We end up creating and keeping around a *lot* of MethodLocation objects
-    // when building a new dex file, so it's worth the trouble of lazily creating
-    // the labels and debugItems lists only when they are needed
-
+    private final LocatedItems<Label> labels;
     @Nullable
-    private List<Label> labels = null;
-    @Nullable
-    private List<BuilderDebugItem> debugItems = null;
+    private final LocatedItems<BuilderDebugItem> debugItems;
 
     MethodLocation(@Nullable BuilderInstruction instruction, int codeAddress, int index) {
+        this.debugItems = new LocatedDebugItems();
+        this.labels = new LocatedLabels();
         this.instruction = instruction;
         this.codeAddress = codeAddress;
         this.index = index;
@@ -74,166 +70,26 @@ public class MethodLocation {
         return index;
     }
 
-    @Nonnull
-    private List<Label> getMutableLabels() {
-        if (labels == null) {
-            labels = new ArrayList<>(1);
-        }
-        return labels;
-    }
-    
-    @Nonnull
-    private List<Label> getImmutableLabels() {
-        if (labels == null) {
-            return ImmutableList.of();
-        }
-        return labels;
-    }
-
-    @Nonnull
-    private List<BuilderDebugItem> getMutableDebugItems() {
-        if (debugItems == null) {
-            debugItems = new ArrayList<>(1);
-        }
-        return debugItems;
-    }
-
-    @Nonnull
-    private List<BuilderDebugItem> getImmutableDebugItems() {
-        if (debugItems == null) {
-            return ImmutableList.of();
-        }
-        return debugItems;
-    }
-
-    @Nonnull
-    private List<BuilderDebugItem> getDebugItems(boolean mutable) {
-        if (debugItems == null) {
-            if (mutable) {
-                debugItems = new ArrayList<>(1);
-                return debugItems;
-            }
-            return ImmutableList.of();
-        }
-        return debugItems;
-    }
-
     void mergeInto(@Nonnull MethodLocation other) {
-        if (this.labels != null || other.labels != null) {
-            List<Label> otherLabels = other.getMutableLabels();
-            for (Label label: this.getImmutableLabels()) {
-                label.location = other;
-                otherLabels.add(label);
-            }
-            this.labels = null;
-        }
-
-        if (this.debugItems != null || other.labels != null) {
-            // We need to keep the debug items in the same order. We add the other debug items to this list, then reassign
-            // the list.
-            List<BuilderDebugItem> debugItems = getMutableDebugItems();
-            for (BuilderDebugItem debugItem: debugItems) {
-                debugItem.location = other;
-            }
-            debugItems.addAll(other.getImmutableDebugItems());
-            other.debugItems = debugItems;
-            this.debugItems = null;
-        }
+        labels.mergeItemsInto(other, other.labels);
+        debugItems.mergeItemsInto(other, other.debugItems);
     }
 
     @Nonnull
     public Set<Label> getLabels() {
-        return new AbstractSet<Label>() {
-            @Nonnull
-            @Override public Iterator<Label> iterator() {
-                final Iterator<Label> it = getImmutableLabels().iterator();
-
-                return new Iterator<Label>() {
-                    private @Nullable Label currentLabel = null;
-
-                    @Override public boolean hasNext() {
-                        return it.hasNext();
-                    }
-
-                    @Override public Label next() {
-                        currentLabel = it.next();
-                        return currentLabel;
-                    }
-
-                    @Override public void remove() {
-                        if (currentLabel != null) {
-                            currentLabel.location = null;
-                        }
-                        it.remove();
-                    }
-                };
-            }
-
-            @Override public int size() {
-                return getImmutableLabels().size();
-            }
-
-            @Override public boolean add(@Nonnull Label label) {
-                if (label.isPlaced()) {
-                    throw new IllegalArgumentException("Cannot add a label that is already placed. You must remove " +
-                            "it from its current location first.");
-                }
-                label.location = MethodLocation.this;
-                getMutableLabels().add(label);
-                return true;
-            }
-        };
+        return labels.getModifiableItems(MethodLocation.this);
     }
 
     @Nonnull
     public Label addNewLabel() {
-        Label label = new Label(this);
-        getMutableLabels().add(label);
-        return label;
+        Label newLabel = new Label();
+        getLabels().add(newLabel);
+        return newLabel;
     }
 
     @Nonnull
     public Set<BuilderDebugItem> getDebugItems() {
-        return new AbstractSet<BuilderDebugItem>() {
-            @Nonnull
-            @Override public Iterator<BuilderDebugItem> iterator() {
-                final Iterator<BuilderDebugItem> it = getImmutableDebugItems().iterator();
-
-                return new Iterator<BuilderDebugItem>() {
-                    private @Nullable BuilderDebugItem currentDebugItem = null;
-
-                    @Override public boolean hasNext() {
-                        return it.hasNext();
-                    }
-
-                    @Override public BuilderDebugItem next() {
-                        currentDebugItem = it.next();
-                        return currentDebugItem;
-                    }
-
-                    @Override public void remove() {
-                        if (currentDebugItem != null) {
-                            currentDebugItem.location = null;
-                        }
-                        it.remove();
-                    }
-                };
-            }
-
-            @Override public int size() {
-                return getImmutableDebugItems().size();
-            }
-
-            @Override public boolean add(@Nonnull BuilderDebugItem debugItem) {
-                if (debugItem.location != null) {
-                    throw new IllegalArgumentException("Cannot add a debug item that has already been added to a " +
-                            "method. You must remove it from its current location first.");
-                }
-                debugItem.location = MethodLocation.this;
-                getMutableDebugItems().add(debugItem);
-                return true;
-            }
-        };
+        return debugItems.getModifiableItems(MethodLocation.this);
     }
 
     public void addLineNumber(int lineNumber) {