Changes between Version 1 and Version 2 of Ticket #16995, comment 17


Ignore:
Timestamp:
2018-11-23T09:44:21+01:00 (7 years ago)
Author:
cmuelle8

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #16995, comment 17

    v1 v2  
    121121However, I'm not quite sure, which of these types is best suited here.  Given that [https://stackoverflow.com/questions/4681960/how-many-bytes-of-memory-does-a-java-util-date-object-use a Date object occupies 32 bytes of memory] it may be more memory efficient to use {{{Date}}} than to store {{{String}}} object to hold e.g. {{{2018-08-01T10:00:00.000Z}}} that [https://www.javamex.com/tutorials/memory/string_memory_usage.shtml according to javamex.com's string_memory_usage article] uses a minimum of 88 bytes in memory (per waypoint!).
    122122
    123 
    124 * So if we are to use {{{Date}}} exclusively for the {{{GpxConstants.PT_TIME}}} attribute in {{{Waypoint.java}}}, we do not need to change the test assertions - neither in the old nor in the proposed test above.
     123EDIT: Below are three alternatives to properly fix the issue, but depending on if and how the {{{WayPoint}}} class is used in plugins, each may trigger work to do afterwards.  Trying to avoid this is the _sole_ reason to rather depend on the patch with the quickfix given above.
     124
     125-----
     126
     127* So if we are to use {{{Date}}} exclusively for the {{{GpxConstants.PT_TIME}}} attribute in {{{Waypoint.java}}}, we do not need to change the test assertions - neither in the old nor in the proposed test above. Patch to use Date exclusively:
     128{{{
     129#!diff
     130--- src/org/openstreetmap/josm/data/gpx/WayPoint.java   (revision 14444)
     131+++ src/org/openstreetmap/josm/data/gpx/WayPoint.java   (working copy)
     132@@ -1,8 +1,9 @@
     133 // License: GPL. For details, see LICENSE file.
     134 package org.openstreetmap.josm.data.gpx;
     135 
     136+import static org.junit.Assert.assertTrue;
     137+
     138 import java.awt.Color;
     139-import java.time.DateTimeException;
     140 import java.util.ArrayList;
     141 import java.util.Date;
     142 import java.util.List;
     143@@ -14,8 +15,6 @@
     144 import org.openstreetmap.josm.data.osm.search.SearchCompiler.Match;
     145 import org.openstreetmap.josm.data.projection.Projecting;
     146 import org.openstreetmap.josm.tools.Logging;
     147-import org.openstreetmap.josm.tools.UncheckedParseException;
     148-import org.openstreetmap.josm.tools.date.DateUtils;
     149 import org.openstreetmap.josm.tools.template_engine.TemplateEngineDataProvider;
     150 
     151 /**
     152@@ -135,7 +134,7 @@
     153      */
     154     public void setTime(Date time) {
     155         this.time = time.getTime() / 1000.;
     156-        this.attr.put(PT_TIME, DateUtils.fromDate(time));
     157+        this.attr.put(PT_TIME, time);
     158     }
     159 
     160     /**
     161@@ -155,8 +154,7 @@
     162      * @since 13210
     163      */
     164     public void setTime(long ts) {
     165-        this.time = ts;
     166-        this.attr.put(PT_TIME, DateUtils.fromTimestamp(ts));
     167+        setTimeInMillis(ts*1000);
     168     }
     169 
     170     /**
     171@@ -167,7 +165,7 @@
     172      */
     173     public void setTimeInMillis(long ts) {
     174         this.time = ts / 1000.;
     175-        this.attr.put(PT_TIME, DateUtils.fromTimestampInMillis(ts));
     176+        this.attr.put(PT_TIME, new Date(ts));
     177     }
     178 
     179     /**
     180@@ -179,10 +177,11 @@
     181         if (attr.containsKey(PT_TIME)) {
     182             try {
     183                 final Object obj = get(PT_TIME);
     184-                final Date date = obj instanceof Date ? (Date) obj : DateUtils.fromString(obj.toString());
     185+                assertTrue(obj instanceof Date);
     186+                final Date date = (Date) obj;
     187                 time = date.getTime() / 1000.;
     188                 return date;
     189-            } catch (UncheckedParseException | DateTimeException e) {
     190+            } catch (AssertionError e) {
     191                 Logging.warn(e);
     192                 time = 0;
     193             }
     194}}}
    125195
    126196* Should we use {{{String}}} objects exclusively the test assertions would need to change.  In this case the strings could be
    127   * either compared directly, e.g. if the {{{DateUtils.getGpxFormat()}}} formatter was used to store the strings in {{{Waypoint.java}}}
    128   * or by converting {{{String}}} to {{{Date}}} first, i.e. {{{gpxFormat.format(DateUtils.fromString(p1.get(GpxConstants.PT_TIME)))}}}
     197
     198  * either compared directly, e.g. if the {{{DateUtils.getGpxFormat()}}} formatter was used to store the strings in {{{Waypoint.java}}}:
     199{{{
     200#!diff
     201--- src/org/openstreetmap/josm/data/gpx/WayPoint.java   (revision 14444)
     202+++ src/org/openstreetmap/josm/data/gpx/WayPoint.java   (working copy)
     203@@ -135,7 +135,7 @@
     204      */
     205     public void setTime(Date time) {
     206         this.time = time.getTime() / 1000.;
     207-        this.attr.put(PT_TIME, DateUtils.fromDate(time));
     208+        this.attr.put(PT_TIME, DateUtils.getGpxFormat().format(time));
     209     }
     210 
     211     /**
     212@@ -155,8 +155,7 @@
     213      * @since 13210
     214      */
     215     public void setTime(long ts) {
     216-        this.time = ts;
     217-        this.attr.put(PT_TIME, DateUtils.fromTimestamp(ts));
     218+        setTimeInMillis(ts*1000);
     219     }
     220 
     221     /**
     222@@ -167,7 +166,7 @@
     223      */
     224     public void setTimeInMillis(long ts) {
     225         this.time = ts / 1000.;
     226-        this.attr.put(PT_TIME, DateUtils.fromTimestampInMillis(ts));
     227+        this.attr.put(PT_TIME, DateUtils.getGpxFormat().format(new Date(ts)));
     228     }
     229 
     230     /**
     231}}}
     232
     233  * or by converting {{{String}}} to {{{Date}}} first, i.e. {{{gpxFormat.format(DateUtils.fromString(p1.get(GpxConstants.PT_TIME)))}}} with no changes {{{Waypoint.java}}} (i.e. PT_TIME attributes stored as {{{String}}}, but not having been normalized by the {{{DateUtils.getGpxFormat()}}} formatter):
     234{{{
     235#!diff
     236--- test/unit/org/openstreetmap/josm/gui/layer/OsmDataLayerTest.java    (revision 14444)
     237+++ test/unit/org/openstreetmap/josm/gui/layer/OsmDataLayerTest.java    (working copy)
     238@@ -220,9 +220,13 @@
     239                 "    <tag k='ele' v='456' />\n" +
     240                 "    <tag k='time' v='2018-08-01T10:01:00Z' />\n" +
     241                 "  </node>\n" +
     242-                "  <way id='-546308'>\n" +
     243+                "  <node id='-546308' timestamp='2018-08-01T10:02:00Z' lat='47.05' lon='9.05'>\n" +
     244+                "    <tag k='ele' v='789' />\n" +
     245+                "  </node>\n" +
     246+                "  <way id='-546309'>\n" +
     247                 "    <nd ref='-546306' />\n" +
     248                 "    <nd ref='-546307' />\n" +
     249+                "    <nd ref='-546308' />\n" +
     250                 "  </way>\r\n" +
     251                 "</osm>").getBytes(StandardCharsets.UTF_8)), null));
     252         GpxData gpx = layer.toGpxData();
     253@@ -237,17 +241,21 @@
     254         Collection<GpxTrackSegment> segments = track.getSegments();
     255         assertEquals(1, segments.size());
     256         Collection<WayPoint> trackpoints = segments.iterator().next().getWayPoints();
     257-        assertEquals(2, trackpoints.size());
     258+        assertEquals(3, trackpoints.size());
     259         Iterator<WayPoint> it = trackpoints.iterator();
     260         DateFormat gpxFormat = DateUtils.getGpxFormat();
     261         WayPoint p1 = it.next();
     262         assertEquals(new LatLon(47.0, 9.0), p1.getCoor());
     263         assertEquals("123", p1.get(GpxConstants.PT_ELE));
     264-        assertEquals("2018-08-01T10:00:00.000Z", gpxFormat.format(p1.get(GpxConstants.PT_TIME)));
     265+        assertEquals("2018-08-01T10:00:00.000Z", gpxFormat.format(DateUtils.fromString((String) p1.get(GpxConstants.PT_TIME))));
     266         WayPoint p2 = it.next();
     267         assertEquals(new LatLon(47.1, 9.1), p2.getCoor());
     268         assertEquals("456", p2.get(GpxConstants.PT_ELE));
     269-        assertEquals("2018-08-01T10:01:00.000Z", gpxFormat.format(p2.get(GpxConstants.PT_TIME)));
     270+        assertEquals("2018-08-01T10:01:00.000Z", gpxFormat.format(DateUtils.fromString((String) p2.get(GpxConstants.PT_TIME))));
     271+        WayPoint p3 = it.next();
     272+        assertEquals(new LatLon(47.05, 9.05), p3.getCoor());
     273+        assertEquals("789", p3.get(GpxConstants.PT_ELE));
     274+        assertEquals("2018-08-01T10:02:00.000Z", gpxFormat.format(DateUtils.fromString((String) p3.get(GpxConstants.PT_TIME))));
     275     }
     276 
     277     /**
     278
     279}}}