[josm-dev] [PATCH] timestamp & parsedTimestamp in OsmPrimitive

[Dirk Stöcker]

On Sun, 8 Mar 2009, Jiri Klement wrote:

> Attached patch replaces timestamp and parsedTimestamp in OsmPrimitive
> with DateContainer class accessible only using setter and getter. The
> patch is not much usefull on itself but hopefully its the first step
> to make OsmPrimitive encalupsed (without public variables) so things
> like spatial index or better caching will be possible in future.

Generally ok, but some points you should fix:

a) I hate constructs like this:
   if (n.getTimestamp().getAsString() != null)
   {
     wpt.attr.put("time", n.getTimestamp().getAsString());

Calling functions twice is always bad design for multiple reasons. Use a 
temporary variable here.

b) This whole block should be in the new class. It's pure time logic.
   String timestr = p.getString("time");
   if(timestr != null)
   {
     timestr = timestr.replace("Z","+00:00");
     n.setTimestamp(new DateContainer(timestr.replace("Z","+00:00")));
   }

c) Same for this one.
   public int compareTo(HistoryItem o) {
     return unifyDate(osm.getTimestamp().getAsDate()).compareTo(unifyDate(o.osm.getTimestamp().getAsDate()));

d) And this one (using getAsNonZeroDate() or the like):
    Date otherd = other.getTimestamp().getAsDate();
    if (otherd == null) {
        otherd = new Date(0);
    }

Ciao
-- 
http://www.dstoecker.eu/ (PGP key available)