On Mon, 9 Mar 2009, Jiri Klement wrote:

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?

Code will be changed later. When a variable should represent a value then call it once or changes may miss the second call. Also it is not sure that compilers optimize the two calls into one and some other reasons also apply which are of minor importance.

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.

No. Actually that is a workaround to convert "Z" (i.e. Zulu time) to offset form, because GPX files have Zulu representation very often.

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.

I though it is clear what I meant:

It should be return osm.getTimestamp().compareTo(o.osm.getTimestamp());

Whatever conversion needs to be done must be inside the date class.
I do not know what unifyDate() does, but surely it is meant to have proper time checking only.

Ciao
--
http://www.dstoecker.eu/ (PGP key available)
_______________________________________________
josm-dev mailing list
josm-dev@openstreetmap.org
http://lists.openstreetmap.org/listinfo/josm-dev

Reply via email to