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

[Dirk Stöcker]

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)