Improve "bogus name" handling when setting StreetEdge's name during postprocessing

[leonardehrenfried]

Summary

The method StreetEdge#setName is called during post-processing and updates the edge's name. This happens only during the "custom naming" phase where new names are assigned to those which where previous marked as having a "bogus name".

A bogus name means that its name is derived from its attributes, so it's "tunnel" or "path".

For this reason I'm changing the code so that the bogus name property is set to false when the name is set.

The final goal is for the mapping from edges to steps to introduce a new step when it encounters such a renamed edge. Currently, this doesn't happen when two edges with "bogus names" are encountered consecutively.

Unit tests

Added.

Documentation

Javadoc around the concept of a bogus name updated.

cc @hbruch @stadtnavimanager

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.78%. Comparing base (a866234) to head (f0dd4f2).
Report is 35 commits behind head on dev-2.x.

Additional details and impacted files

@@              Coverage Diff              @@
##             dev-2.x    #6321      +/-   ##
=============================================
- Coverage      69.79%   69.78%   -0.01%     
+ Complexity     17812    17811       -1     
=============================================
  Files           2020     2020              
  Lines          76221    76222       +1     
  Branches        7799     7799              
=============================================
  Hits           53195    53195              
  Misses         20312    20312              
- Partials        2714     2715       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[abyrd]

These changes are a great improvement to comprehension and readability, and could therefore be merged as-is. It is worth considering though whether we should switch to a term other than "bogus" in a follow-up commit.

[abyrd]

The expression "has bogus name" in the context of an accessor method may imply that a "bogus name" is a thing that may be present or absent, in addition to a "normal name". Seeing these terms in the past I am not sure I realized that "bogusness" was only signalling the provenance of the name, not an extra name. If we want to keep using the term "bogus", is it a better description to say "Name is bogus" instead of "we have a bogus name"?

Taking this a step further, is there a more descriptive term for this than "bogus"? The names are more "inferred", "generic", or "placeholder" than "bogus". Or inverting the boolean condition, "normal" names are those explicitly specified by OSM name tags. Unfortunately "has explicit name from tag" is a little long. NAME_IS_GENERIC or NAME_IS_INFERRED?

[leonardehrenfried]

I fully agree with this but I want to do it in another PR.

I prefer NAME_IS_INFERRED. We can also discuss if the boolean is the right way to model this or if getName should return a sealed interface to express the two types of names.

[leonardehrenfried]

I did a bit of code archeology and I believe that this is the commit that introduced "bogus names": c1eedb1

The relevant part is this:

    /**
     * The name of this street was generated by the system, so we should only display it once, and 
     * generally just display right/left directions
     */

[habrahamsson-skanetrafiken]

I agree that the "bogus name" is a poor choice and it would be nice to change it at some point.

But this PR is a good improvement so I think it should be merged.

[abyrd]

I did a bit of code archeology and I believe that this is the commit that introduced "bogus names": [c1eedb1]

The PR title is "Mark bogus (generated) edge names in the itinterary" so even the original commit included an alternative clarifying term: "Generated".