s2geometry icon indicating copy to clipboard operation
s2geometry copied to clipboard

Types and functions should be in a namespace or consistently prefixed to avoid ODR violations

Open joka921 opened this issue 4 months ago • 1 comments

Currently, the S2 library is not wrapped in a namespace. Additionally, most of the class names contain "S2" somewhere in their name, but not all of them. In particular, there are types called Decoder and Encoder in the public namespace defined in util/coding/coder.h. This leads to one-definition-rule violations when S2 is linked together with other code that also uses classes of the same name (in fact I ran into such problems).

In my opinion, there are two possibilities to mitigate this problem:

  1. Consistently use a namespace for everything inside S2.
  2. Consistently use "S2" as a prefix, suffix, or infix of all names of type, i.e renaming the Encoder and Decoder classes to S2Encoder and S2Decoder or using a similar naming scheme.

Please let me know, which variant you would prefer, and whether I should prepare a PR or whether you want to fix this yourselves, as it probably would break existing code and would require adaptations in documentations etc.

joka921 avatar Oct 07 '24 09:10 joka921