Hi there, examining the code of mkgmap i found some strange lines. In /mkgmap/src/uk/me/parabola/util/ShapeSplitter.java Line 330: This is redundant? In /mkgmap/src/uk/me/parabola/mkgmap/osmstyle/housenumber/HousenumberGenerator.java Line 1537+1538: This was already tested 6 lines above, so this cannot occur - Dead code? In /mkgmap/src/uk/me/parabola/mkgmap/filters/ShapeMergeFilter.java Line 554: This is a String comparison, shouldn't *n1.equals(n2)* be used instead? Just some thoughts of mine browsing the code - your opinions? -- View this message in context: http://gis.19327.n8.nabble.com/Strange-code-in-mkgmap-tp5899307.html Sent from the Mkgmap Development mailing list archive at Nabble.com.
The ShapeSplitter code looks like my mistake - I suspect it should be testing the lowPoint and highPoint, but need to think about this more carefully before committing myself. HousenumberGenerator not mine but - yes, looks like those 2 lines should be removed ShapeMergeFilter isn't mine, but looks OK. For efficiency it first tests if the names are the same object, then null/string ordering, finally a string comparison. Ticker On Sat, 2017-07-15 at 10:49 -0700, UliBaer wrote:
Hi there,
examining the code of mkgmap i found some strange lines.
In /mkgmap/src/uk/me/parabola/util/ShapeSplitter.java Line 330: This is redundant?
In /mkgmap/src/uk/me/parabola/mkgmap/osmstyle/housenumber/HousenumberGen erator.java Line 1537+1538: This was already tested 6 lines above, so this cannot occur - Dead code?
In /mkgmap/src/uk/me/parabola/mkgmap/filters/ShapeMergeFilter.java Line 554: This is a String comparison, shouldn't *n1.equals(n2)* be used instead?
Just some thoughts of mine browsing the code - your opinions?
-- View this message in context: http://gis.19327.n8.nabble.com/Strange-code-in-mkgmap-tp5899307.html Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hi Gerd Attached is patch for ShapeSplitter Ticker On Sat, 2017-07-15 at 19:40 +0100, Ticker Berkin wrote:
The ShapeSplitter code looks like my mistake - I suspect it should be testing the lowPoint and highPoint, but need to think about this more carefully before committing myself.
HousenumberGenerator not mine but - yes, looks like those 2 lines should be removed
ShapeMergeFilter isn't mine, but looks OK. For efficiency it first tests if the names are the same object, then null/string ordering, finally a string comparison.
Ticker
On Sat, 2017-07-15 at 10:49 -0700, UliBaer wrote:
Hi there,
examining the code of mkgmap i found some strange lines.
In /mkgmap/src/uk/me/parabola/util/ShapeSplitter.java Line 330: This is redundant?
In /mkgmap/src/uk/me/parabola/mkgmap/osmstyle/housenumber/HousenumberG en erator.java Line 1537+1538: This was already tested 6 lines above, so this cannot occur - Dead code?
In /mkgmap/src/uk/me/parabola/mkgmap/filters/ShapeMergeFilter.java Line 554: This is a String comparison, shouldn't *n1.equals(n2)* be used instead?
Just some thoughts of mine browsing the code - your opinions?
-- View this message in context: http://gis.19327.n8.nabble.com/Strange-code-in-mkgmap-tp5899307.htm l Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hi Ticker, thanks, seems that Uil found an error here. Can you add a unit test that shows where this code is relevant? Gerd ________________________________________ Von: mkgmap-dev <mkgmap-dev-bounces@lists.mkgmap.org.uk> im Auftrag von Ticker Berkin <rwb-mkgmap@jagit.co.uk> Gesendet: Mittwoch, 26. Juli 2017 12:12:36 An: mkgmap-dev@lists.mkgmap.org.uk Betreff: Re: [mkgmap-dev] Strange code in mkgmap? Hi Gerd Attached is patch for ShapeSplitter Ticker On Sat, 2017-07-15 at 19:40 +0100, Ticker Berkin wrote:
The ShapeSplitter code looks like my mistake - I suspect it should be testing the lowPoint and highPoint, but need to think about this more carefully before committing myself.
HousenumberGenerator not mine but - yes, looks like those 2 lines should be removed
ShapeMergeFilter isn't mine, but looks OK. For efficiency it first tests if the names are the same object, then null/string ordering, finally a string comparison.
Ticker
On Sat, 2017-07-15 at 10:49 -0700, UliBaer wrote:
Hi there,
examining the code of mkgmap i found some strange lines.
In /mkgmap/src/uk/me/parabola/util/ShapeSplitter.java Line 330: This is redundant?
In /mkgmap/src/uk/me/parabola/mkgmap/osmstyle/housenumber/HousenumberG en erator.java Line 1537+1538: This was already tested 6 lines above, so this cannot occur - Dead code?
In /mkgmap/src/uk/me/parabola/mkgmap/filters/ShapeMergeFilter.java Line 554: This is a String comparison, shouldn't *n1.equals(n2)* be used instead?
Just some thoughts of mine browsing the code - your opinions?
-- View this message in context: http://gis.19327.n8.nabble.com/Strange-code-in-mkgmap-tp5899307.htm l Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
Hi Gerd I don't think this needs a unit test. The patch is just to fix a copy/paste error. This code deals with a pathological case where there is a cut through a shape that loops around and along itself (3 lines on top of each other) and to distinguish whether it is a hole looping back inside or an area looping back outside. Ticker On Thu, 2017-07-27 at 06:32 +0000, Gerd Petermann wrote:
Hi Ticker,
thanks, seems that Uil found an error here. Can you add a unit test that shows where this code is relevant?
Gerd ________________________________________ Von: mkgmap-dev <mkgmap-dev-bounces@lists.mkgmap.org.uk> im Auftrag von Ticker Berkin <rwb-mkgmap@jagit.co.uk> Gesendet: Mittwoch, 26. Juli 2017 12:12:36 An: mkgmap-dev@lists.mkgmap.org.uk Betreff: Re: [mkgmap-dev] Strange code in mkgmap?
Hi Gerd
Attached is patch for ShapeSplitter
Ticker
On Sat, 2017-07-15 at 19:40 +0100, Ticker Berkin wrote:
The ShapeSplitter code looks like my mistake - I suspect it should be testing the lowPoint and highPoint, but need to think about this more carefully before committing myself.
HousenumberGenerator not mine but - yes, looks like those 2 lines should be removed
ShapeMergeFilter isn't mine, but looks OK. For efficiency it first tests if the names are the same object, then null/string ordering, finally a string comparison.
Ticker
On Sat, 2017-07-15 at 10:49 -0700, UliBaer wrote:
Hi there,
examining the code of mkgmap i found some strange lines.
In /mkgmap/src/uk/me/parabola/util/ShapeSplitter.java Line 330: This is redundant?
In /mkgmap/src/uk/me/parabola/mkgmap/osmstyle/housenumber/Housenumbe rG en erator.java Line 1537+1538: This was already tested 6 lines above, so this cannot occur - Dead code?
In /mkgmap/src/uk/me/parabola/mkgmap/filters/ShapeMergeFilter.java Line 554: This is a String comparison, shouldn't *n1.equals(n2)* be used instead?
Just some thoughts of mine browsing the code - your opinions?
-- View this message in context: http://gis.19327.n8.nabble.com/Strange-code-in-mkgmap-tp5899307.h tm l Sent from the Mkgmap Development mailing list archive at Nabble.com. _______________________________________________ mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
mkgmap-dev mailing list mkgmap-dev@lists.mkgmap.org.uk http://www.mkgmap.org.uk/mailman/listinfo/mkgmap-dev
participants (3)
-
Gerd Petermann -
Ticker Berkin -
UliBaer