[OSM-dev] Additions to Osmosis

Brett Henderson brett at bretth.com
Tue Nov 6 02:03:38 GMT 2007


Karl Newman wrote:
> Well, the way I have it planned it's behavior will be pretty
> unsophisticated--if the input data has a bounds element, use it as the
> starting point (for databases, maybe assume entire planet?) For
> bounding boxes, just take the intersection of the existing bounds and
> the bounding box. For polygons, I'd just use the enclosing rectangular
> extents of the resultant Area. For merge operations, use the union of
> the two bboxes.
>   
It all depends on who's consuming these bounds tags.  If they're purely 
informational then your approach is fine.  If they're being used by 
downstream applications then that may change things.  Given that osmosis 
doesn't currently support them at all, your approach sounds like a good 
start.  If anybody screams later, then we can look at how to improve it.
>> I didn't worry about any of this in the initial implementation because I
>> didn't see the need for it.  But if it's something you can use, then by
>> all means implement it.  One minor thing to be aware of is that the
>> current implementation of bounding box is fairly lenient in its command
>> line arguments, if left > right it automatically switches the arguments,
>> this is something Marcus added fairly recently.  If this breaks your
>> implementation then it sounds reasonable to make them more strict again.
>>
>>     
>
> I don't strictly have a need for it, but I thought it would be good to
> make the feature complete.
>
>   
>> It may be a good time to rename some of the command line arguments as
>> well, currently it uses left, right, top and bottom, perhaps minLat,
>> minLon, etc would be more appropriate.  Whatever we choose should
>> attempt to align with any defacto standards used elsewhere in OSM.
>> Marcus was keen to see these arguments renamed because it's not very
>> clear what type of arguments left, right, etc are.
>>
>> In summary, my initial implementation was very simple so any
>> improvements are welcome.
>>     
>
> Maybe east, west, north, south? I'm hesitant about "min" and "max" for
> the very reason that the bounding box may span the 180th meridian,
> where the coordinates of the "west" boundary would be greater than the
> coordinates for the "east" boundary. It confuses the idea of min and
> max.
>   
I don't have a strong opinion on this one.  It's not important so we can 
worry about it later.  These arguments are one of the less confusing 
aspects of osmosis usage.
>> One final comment.  If you do find yourself changing the behaviour of
>> existing tasks, it may be worth creating new tasks instead.
>>     
>
> How much change is tolerable before new tasks are required? The bounds
> element should be a pretty simple change that most tasks will ignore
> (defer to the base class). The 180th meridian addition would affect
> the two area filter subclasses, and in the degenerate case would
> collapse to the current behavior (maybe a strategy pattern would be
> appropriate?). I'm not sure it would make sense to create new tasks in
> that case, but if you think otherwise, that's fine. The rules filter
> would certainly be a task of its own.
>   
I generally try to avoid two things:
* Large numbers of fragile conditional statements in task logic.  
Primarily so I can understand the code a month later, but also because 
it tends to improve performance.
* Breaking existing osmosis users.

Neither have been a major issue up to this point.  In most cases where 
I've changed behaviour, I've added parameters to make the new behaviour 
optional (as you already have with the completeWays and 
completeRelations arguments to the area tasks).  For new functionality 
such as polygon support I refactored most of the existing bounding box 
task into a common parent class so they could share most of their 
functionality.

In your case it sounds like you're improving existing behaviour so it 
probably makes sense to enhance existing tasks.  A strategy pattern is 
the way I was leaning with your earlier enhancement of the area filter 
tasks to make the behaviour runtime selectable, something similar to the 
way the sort tasks allow different orderings to be specified.  In the 
area filter case it seemed like a bit of work for no return so I didn't 
bother.  If the tasks get more complicated we can revisit it.  Do 
whatever you wish for now, the hard bit is getting something working, 
restructuring is usually fairly straightforward.

Cheers,
Brett





More information about the dev mailing list