On Sun, Mar 8, 2009 at 11:17 PM, Dirk Stöcker <[email protected]> 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
> [email protected]
> http://lists.openstreetmap.org/listinfo/josm-dev
>
timestamp.patch
Description: Binary data
_______________________________________________ josm-dev mailing list [email protected] http://lists.openstreetmap.org/listinfo/josm-dev
