Changeset 11646 in josm

Timestamp:

2017-03-02T00:19:17+01:00 (8 years ago)

Author:

Don-vip

Message:

see #14175 - refactor HistoryBrowserModel

Location:

trunk

Files:

• src/org/openstreetmap/josm/gui/history/HistoryBrowserDialog.java (modified) (2 diffs)
• src/org/openstreetmap/josm/gui/history/HistoryBrowserModel.java (modified) (20 diffs)
• src/org/openstreetmap/josm/gui/history/VersionTable.java (modified) (5 diffs)
• src/org/openstreetmap/josm/gui/history/VersionTableModel.java (added)
• test/unit/org/openstreetmap/josm/gui/history/HistoryBrowserModelTest.java (modified) (1 diff)

trunk/src/org/openstreetmap/josm/gui/history/HistoryBrowserDialog.java

@@ -104 +104 @@
 
         btn = new JButton(closeAction);
-        final String closeHistoryBrowserDialogKey = "CloseHistoryBrowserDialog";
         btn.setName("btn.close");
         pnl.add(btn);
@@ -158 +157 @@
         }
 
-        public void run() {
+        void run() {
             getHistoryBrowser().getModel().unlinkAsListener();
             HistoryDataSet.getInstance().removeHistoryDataSetListener(HistoryBrowserDialog.this);

trunk/src/org/openstreetmap/josm/gui/history/HistoryBrowserModel.java

@@ -4 +4 @@
 import static org.openstreetmap.josm.tools.I18n.tr;
 
-import java.text.DateFormat;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -16 +15 @@
 
 import org.openstreetmap.josm.Main;
-import org.openstreetmap.josm.data.osm.Changeset;
 import org.openstreetmap.josm.data.osm.Node;
 import org.openstreetmap.josm.data.osm.OsmPrimitive;
@@ -48 +46 @@
 import org.openstreetmap.josm.gui.util.ChangeNotifier;
 import org.openstreetmap.josm.tools.CheckParameterUtil;
-import org.openstreetmap.josm.tools.date.DateUtils;
 
 /**
@@ -98 +95 @@
      */
     public HistoryBrowserModel() {
-        versionTableModel = new VersionTableModel();
+        versionTableModel = new VersionTableModel(this);
         currentTagTableModel = new TagTableModel(PointInTimeType.CURRENT_POINT_IN_TIME);
         referenceTagTableModel = new TagTableModel(PointInTimeType.REFERENCE_POINT_IN_TIME);
@@ -135 +132 @@
     }
 
-    protected boolean canShowAsLatest(OsmPrimitive primitive) {
+    private boolean canShowAsLatest(OsmPrimitive primitive) {
         if (primitive == null)
             return false;
@@ -197 +194 @@
     }
 
-    protected void fireModelChange() {
+    private void fireModelChange() {
         initNodeListTableModels();
         initMemberListTableModels();
@@ -214 +211 @@
     }
 
-    protected void initTagTableModels() {
+    private void initTagTableModels() {
         currentTagTableModel.initKeyList();
         referenceTagTableModel.initKeyList();
@@ -223 +220 @@
      * TODO: Maybe rename to reflect this? eg. updateNodeListTableModels
      */
-    protected void initNodeListTableModels() {
+    private void initNodeListTableModels() {
         if (current == null || current.getType() != OsmPrimitiveType.WAY
          || reference == null || reference.getType() != OsmPrimitiveType.WAY)
@@ -234 +231 @@
     }
 
-    protected void initMemberListTableModels() {
+    private void initMemberListTableModels() {
         if (current == null || current.getType() != OsmPrimitiveType.RELATION
          || reference == null || reference.getType() != OsmPrimitiveType.RELATION)
@@ -397 +394 @@
 
     /**
-     * The table model for the list of versions in the current history
-     *
-     */
-    public final class VersionTableModel extends AbstractTableModel {
-
-        private VersionTableModel() {
-        }
-
-        @Override
-        public int getRowCount() {
-            if (history == null)
-                return 0;
-            int ret = history.getNumVersions();
+     * Sets the reference point in time to the given row.
+     * @param row row number
+     */
+    public void setReferencePointInTime(int row) {
+        if (history == null)
+            return;
+        if (row == history.getNumVersions()) {
             if (latest != null) {
-                ret++;
-            }
-            return ret;
-        }
-
-        @Override
-        public Object getValueAt(int row, int column) {
-            switch (column) {
-            case VersionTableColumnModel.COL_VERSION:
-                HistoryOsmPrimitive p1 = getPrimitive(row);
-                if (p1 != null)
-                    return Long.toString(p1.getVersion());
-                return null;
-            case VersionTableColumnModel.COL_REFERENCE:
-                return isReferencePointInTime(row);
-            case VersionTableColumnModel.COL_CURRENT:
-                return isCurrentPointInTime(row);
-            case VersionTableColumnModel.COL_DATE:
-                HistoryOsmPrimitive p3 = getPrimitive(row);
-                if (p3 != null && p3.getTimestamp() != null)
-                    return DateUtils.formatDateTime(p3.getTimestamp(), DateFormat.SHORT, DateFormat.SHORT);
-                return null;
-            case VersionTableColumnModel.COL_USER:
-                HistoryOsmPrimitive p4 = getPrimitive(row);
-                if (p4 != null) {
-                    User user = p4.getUser();
-                    if (user != null)
-                        return user.getName();
-                }
-                return null;
-            case VersionTableColumnModel.COL_EDITOR:
-                HistoryOsmPrimitive p5 = getPrimitive(row);
-                if (p5 != null) {
-                    Changeset cs = p5.getChangeset();
-                    if (cs != null) {
-                        return cs.get("created_by");
-                    }
-                }
-                return null;
-            }
+                setReferencePointInTime(latest);
+            }
+            return;
+        }
+        if (row < 0 || row > history.getNumVersions())
+            return;
+        setReferencePointInTime(history.get(row));
+    }
+
+    /**
+     * Sets the current point in time to the given row.
+     * @param row row number
+     */
+    public void setCurrentPointInTime(int row) {
+        if (history == null)
+            return;
+        if (row == history.getNumVersions()) {
+            if (latest != null) {
+                setCurrentPointInTime(latest);
+            }
+            return;
+        }
+        if (row < 0 || row > history.getNumVersions())
+            return;
+        setCurrentPointInTime(history.get(row));
+    }
+
+    /**
+     * Determines if the given row is the reference point in time.
+     * @param row row number
+     * @return {@code true} if the given row is the reference point in time
+     */
+    public boolean isReferencePointInTime(int row) {
+        if (history == null)
+            return false;
+        if (row == history.getNumVersions())
+            return latest == reference;
+        if (row < 0 || row > history.getNumVersions())
+            return false;
+        return history.get(row) == reference;
+    }
+
+    /**
+     * Determines if the given row is the current point in time.
+     * @param row row number
+     * @return {@code true} if the given row is the current point in time
+     */
+    public boolean isCurrentPointInTime(int row) {
+        if (history == null)
+            return false;
+        if (row == history.getNumVersions())
+            return latest == current;
+        if (row < 0 || row > history.getNumVersions())
+            return false;
+        return history.get(row) == current;
+    }
+
+    /**
+     * Returns the {@code HistoryPrimitive} at the given row.
+     * @param row row number
+     * @return the {@code HistoryPrimitive} at the given row
+     */
+    public HistoryOsmPrimitive getPrimitive(int row) {
+        if (history == null)
             return null;
-        }
-
-        @Override
-        public void setValueAt(Object aValue, int row, int column) {
-            if (!((Boolean) aValue))
-                return;
-            switch (column) {
-            case 1:
-                setReferencePointInTime(row);
-                break;
-            case 2:
-                setCurrentPointInTime(row);
-                break;
-            default:
-                return;
-            }
-            fireTableDataChanged();
-        }
-
-        @Override
-        public boolean isCellEditable(int row, int column) {
-            return column >= 1 && column <= 2;
-        }
-
-        public void setReferencePointInTime(int row) {
-            if (history == null)
-                return;
-            if (row == history.getNumVersions()) {
-                if (latest != null) {
-                    HistoryBrowserModel.this.setReferencePointInTime(latest);
-                }
-                return;
-            }
-            if (row < 0 || row > history.getNumVersions())
-                return;
-            HistoryOsmPrimitive reference = history.get(row);
-            HistoryBrowserModel.this.setReferencePointInTime(reference);
-        }
-
-        public void setCurrentPointInTime(int row) {
-            if (history == null)
-                return;
-            if (row == history.getNumVersions()) {
-                if (latest != null) {
-                    HistoryBrowserModel.this.setCurrentPointInTime(latest);
-                }
-                return;
-            }
-            if (row < 0 || row > history.getNumVersions())
-                return;
-            HistoryOsmPrimitive current = history.get(row);
-            HistoryBrowserModel.this.setCurrentPointInTime(current);
-        }
-
-        public boolean isReferencePointInTime(int row) {
-            if (history == null)
-                return false;
-            if (row == history.getNumVersions())
-                return latest == reference;
-            if (row < 0 || row > history.getNumVersions())
-                return false;
-            HistoryOsmPrimitive p = history.get(row);
-            return p == reference;
-        }
-
-        public boolean isCurrentPointInTime(int row) {
-            if (history == null)
-                return false;
-            if (row == history.getNumVersions())
-                return latest == current;
-            if (row < 0 || row > history.getNumVersions())
-                return false;
-            HistoryOsmPrimitive p = history.get(row);
-            return p == current;
-        }
-
-        public HistoryOsmPrimitive getPrimitive(int row) {
-            if (history == null)
-                return null;
-            return isLatest(row) ? latest : history.get(row);
-        }
-
-        public boolean isLatest(int row) {
-            return row >= history.getNumVersions();
-        }
-
-        public OsmPrimitive getLatest() {
-            if (latest == null)
-                return null;
-            OsmDataLayer editLayer = Main.getLayerManager().getEditLayer();
-            if (editLayer == null)
-                return null;
-            return editLayer.data.getPrimitiveById(latest.getId(), latest.getType());
-        }
-
-        @Override
-        public int getColumnCount() {
-            return 6;
-        }
+        return isLatest(row) ? latest : history.get(row);
+    }
+
+    /**
+     * Determines if the given row is the latest.
+     * @param row row number
+     * @return {@code true} if the given row is the latest
+     */
+    public boolean isLatest(int row) {
+        return row >= history.getNumVersions();
+    }
+
+    /**
+     * Returns the latest {@code HistoryOsmPrimitive}.
+     * @return the latest {@code HistoryOsmPrimitive}
+     */
+    public HistoryOsmPrimitive getLatest() {
+        return latest;
     }
 
@@ -558 +492 @@
      *
      */
-    public class TagTableModel extends AbstractTableModel {
+    public final class TagTableModel extends AbstractTableModel {
 
         private List<String> keys;
         private final PointInTimeType pointInTimeType;
 
-        protected TagTableModel(PointInTimeType type) {
+        private TagTableModel(PointInTimeType type) {
             pointInTimeType = type;
             initKeyList();
         }
 
-        protected void initKeyList() {
+        private void initKeyList() {
             Set<String> keySet = new HashSet<>();
             if (current != null) {
@@ -603 +537 @@
         }
 
+        /**
+         * Determines if a tag exists for the given key.
+         * @param key tag key
+         * @return {@code true} if a tag exists for the given key
+         */
         public boolean hasTag(String key) {
             HistoryOsmPrimitive primitive = getPointInTime(pointInTimeType);
@@ -608 +547 @@
         }
 
+        /**
+         * Returns the tag value for the given key.
+         * @param key tag key
+         * @return tag value, or null
+         */
         public String getValue(String key) {
             HistoryOsmPrimitive primitive = getPointInTime(pointInTimeType);
@@ -615 +559 @@
         }
 
+        /**
+         * Determines if a tag exists in the opposite point in time for the given key.
+         * @param key tag key
+         * @return {@code true} if a tag exists for the given key
+         */
         public boolean oppositeHasTag(String key) {
             PointInTimeType opposite = pointInTimeType.opposite();
@@ -621 +570 @@
         }
 
+        /**
+         * Returns the tag value in the opposite point in time for the given key.
+         * @param key tag key
+         * @return tag value, or null
+         */
         public String getOppositeValue(String key) {
             PointInTimeType opposite = pointInTimeType.opposite();
@@ -629 +583 @@
         }
 
+        /**
+         * Determines if the tag value is the same in the opposite point in time for the given key.
+         * @param key tag key
+         * @return {@code true} if the tag value is the same in the opposite point in time for the given key
+         */
         public boolean hasSameValueAsOpposite(String key) {
             String value = getValue(key);
@@ -635 +594 @@
         }
 
+        /**
+         * Returns the type of point in time.
+         * @return the type of point in time
+         */
         public PointInTimeType getPointInTimeType() {
             return pointInTimeType;
         }
 
+        /**
+         * Determines if this is the current point in time.
+         * @return {@code true} if this is the current point in time
+         */
         public boolean isCurrentPointInTime() {
             return pointInTimeType.equals(PointInTimeType.CURRENT_POINT_IN_TIME);
         }
 
+        /**
+         * Determines if this is the reference point in time.
+         * @return {@code true} if this is the reference point in time
+         */
         public boolean isReferencePointInTime() {
             return pointInTimeType.equals(PointInTimeType.REFERENCE_POINT_IN_TIME);
@@ -653 +624 @@
     }
 
+    /**
+     * Sets the latest {@code HistoryOsmPrimitive}.
+     * @param latest the latest {@code HistoryOsmPrimitive}
+     */
     protected void setLatest(HistoryOsmPrimitive latest) {
         if (latest == null) {
@@ -744 +719 @@
             return;
         OsmPrimitive primitive = event.getDataset().getPrimitiveById(history.getId(), history.getType());
-        HistoryOsmPrimitive latest;
+        HistoryOsmPrimitive newLatest;
         if (canShowAsLatest(primitive)) {
-            latest = new HistoryPrimitiveBuilder().build(primitive);
+            newLatest = new HistoryPrimitiveBuilder().build(primitive);
         } else {
-            latest = null;
-        }
-        setLatest(latest);
+            newLatest = null;
+        }
+        setLatest(newLatest);
         fireModelChange();
     }
@@ -825 +800 @@
         }
 
-        public HistoryOsmPrimitive build(OsmPrimitive primitive) {
+        HistoryOsmPrimitive build(OsmPrimitive primitive) {
             primitive.accept(this);
             return clone;

trunk/src/org/openstreetmap/josm/gui/history/VersionTable.java

@@ -58 +58 @@
     }
 
+    /**
+     * Builds the table.
+     */
     protected void build() {
         getTableHeader().setFont(getTableHeader().getFont().deriveFont(9f));
@@ -108 +111 @@
     }
 
-    protected HistoryBrowserModel.VersionTableModel getVersionTableModel() {
-        return (HistoryBrowserModel.VersionTableModel) getModel();
-    }
-
     @Override
     public void stateChanged(ChangeEvent e) {
@@ -129 +128 @@
                 int col = columnAtPoint(e.getPoint());
                 if (row >= 0 && (col == VersionTableColumnModel.COL_DATE || col == VersionTableColumnModel.COL_USER)) {
-                    model.getVersionTableModel().setCurrentPointInTime(row);
-                    model.getVersionTableModel().setReferencePointInTime(Math.max(0, row - 1));
+                    model.setCurrentPointInTime(row);
+                    model.setReferencePointInTime(Math.max(0, row - 1));
                 }
             }
@@ -137 +136 @@
         @Override
         protected int checkTableSelection(JTable table, Point p) {
-            HistoryBrowserModel.VersionTableModel tableModel = getVersionTableModel();
             int row = rowAtPoint(p);
-            if (row > -1 && !tableModel.isLatest(row)) {
-                popupMenu.prepare(tableModel.getPrimitive(row));
+            if (row > -1 && !model.isLatest(row)) {
+                popupMenu.prepare(model.getPrimitive(row));
             }
             return row;
@@ -315 +313 @@
         }
 
-        // for unit tests
-        private AlignedRenderer() {
+        AlignedRenderer() {
             this(SwingConstants.LEFT);
         }

trunk/test/unit/org/openstreetmap/josm/gui/history/HistoryBrowserModelTest.java

@@ -18 +18 @@
 import org.openstreetmap.josm.data.osm.history.HistoryNode;
 import org.openstreetmap.josm.gui.history.HistoryBrowserModel.TagTableModel;
-import org.openstreetmap.josm.gui.history.HistoryBrowserModel.VersionTableModel;
 import org.openstreetmap.josm.testutils.JOSMTestRules;