[mkgmap-dev] refactoring

Mon Apr 21 13:44:13 BST 2014

Hi WanMil,

> > @WanMil:
> > Special case: Code in  StyledConverter near roadLog.isInfoEnabled()
> > formats the content of the TabAAcesss field in RoadDef which
> > contains the img format. I think this is okay because the intention
> > of the log is to show exactly the content of this field.
> 
>  From my point of view it doesn't matter to have the exact bit coding 
> here. The intention of the output was to get an information about the 
> access restrictions. For ease of use I decided to use the bit 
> representation of the map but if another output is easier to code or to 
> read please feel free to change.
Okay. Maybe I'll change that part again as the TabAAcesss field is
also not directly written to the img file.

> 
> >
> > 2) Create class ConvertedWay which combines an
> > OSM Way instance with a reference to a GType instance.
> > Instead of copying and modifying fields (roadClass, roadSpeed)
> > in GType instances this is now done in ConvertedWay.
> > Many tags which are relevant for routing are evaluated once
> > when the ConvertedWay instance is created.
> > The results are kept in bit masks, this allows a quick
> > compare.
> > Also, RoadMerger uses this class instead of creating new
> > Road instances.
> 
> Ok.
> One question regarding line 550:
> if (road1.getRouteFlags() != road2.getRouteFlags())
> If the route flags are different are the ways always not mergable?
> In such a case the code lines after are important for debugging only. 
> Some CPU cycles can be saved by adding if (logger.isDebugEnabled()).
> I also suggest to move the low cost checks to the beginning of 
> isWayMergable(). Before refactoring all checks were working on tags and 
> therefore had the same cost.

yes, roads with different routeFlags  should not be merged.
You are right, the code should only be executed if logging is needed. 

> 
> >
> > @WanMil: The new code produces a different log on debug
> > level because some comparisons are done in different order.
> > I've changed the code to compare the bit mask fields instead of the
> > tags, but the code still lists (most of) the compared tags
> > to be able to produce debug info.
> > Let me know if that is okay for you. Another solution could be to
> > implement the debugging in ConvertedWay e.g. something
> > like
> > public String compareAccess(ConvertedWay other)
> > could return a String like
> > "mkgmap:truck is different 'no' <> 'null' "
> 
> I don't mind about the output as long as it contains the required 
> information in a readable format :-)
Okay. I'll see how complicated this method is. It could
also replace the loops for the comparison of the routeFlags.
> 
> >
> > 3)
> > With r3214 I've renamed a few methods. Please check if
> > you find better names:
> > http://www.mkgmap.org.uk/websvn/revision.php?repname=mkgmap&rev=3214
> 
> some ideas:
> Element.getEntrySetIterator() => Element.getTagIterator()

I used this first, but then I decided that tag usally means
the pair key=value as a single String. 
The Tags class implements that as iterator().
Element.getEntrySetIterator() calls Tags.entryIterator()
so maybe it should better be changed to 
Element.getEntryIterator() 

> 
> 
> 
> Some other comments:
> 
> RestrictionRelation.setExceptMask should be updated to the new access 
> rules. But maybe this change should be applied not in the refactoring 
> branch without any change in the output.
> Example:
> delivery is not an access tag and should be removed
> psv should set bus and taxi (if they are not set too)
> motorcycle should be ignored
> vehicle should be considered.

Yes, I also don't like that part of the code.
I'd prefer to have a rules file for it, but
I found no simple way to code this.
I'll change that after merging to trunk,
the refactoring should not change functionality.

> ...
> 
> HighwayHooks.usedTags should not be static.
> It should be possible to compile one tile with make-opposite-cycleways 
> and another without. So the usedTags field should be different.

Good catch. I've corrected that with r3218.

Gerd

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mkgmap.org.uk/pipermail/mkgmap-dev/attachments/20140421/b476e534/attachment.html>