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

Jiri Klement jiri.klement at gmail.com
Mon Mar 9 07:16:15 GMT 2009


On Sun, Mar 8, 2009 at 11:17 PM, Dirk Stöcker
<openstreetmap at dstoecker.de> wrote:
> 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.

OK, I've fixed that. Way is calling functions twice bad design? Is
OsmPrimitive supposed to be thread safe?

> 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")));
>   }
I don't agree with this one. DateConverter seems to expect date in
multiple different formats while code here seems to be sure that the
date is in some specific format. So I think this is gpx specific.

> c) Same for this one.
>   public int compareTo(HistoryItem o) {
>     return unifyDate(osm.getTimestamp().getAsDate()).compareTo(unifyDate(o.osm.getTimestamp().getAsDate()));
Again, this is imho specific to history dialog.

>
> d) And this one (using getAsNonZeroDate() or the like):
>    Date otherd = other.getTimestamp().getAsDate();
>    if (otherd == null) {
>        otherd = new Date(0);
>    }
OK, I've changed getAsDate to always return non null value. If
somebody is interested whether date is null then he can use isEmpty
method.

> Ciao
> --
> http://www.dstoecker.eu/ (PGP key available)
>
> _______________________________________________
> josm-dev mailing list
> josm-dev at openstreetmap.org
> http://lists.openstreetmap.org/listinfo/josm-dev
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: timestamp.patch
Type: application/octet-stream
Size: 10288 bytes
Desc: not available
URL: <http://lists.openstreetmap.org/pipermail/josm-dev/attachments/20090309/12d2aab8/attachment.obj>


More information about the josm-dev mailing list