Code consisitency/correctness around pointers

[springmeyer]

In regards to

vtquery/src/vtquery.cpp

Lines 26 to 29 in 5ffe8d7

	static const char* GeomTypeStrings[] = {"point", "linestring", "polygon", "unknown"};
	const char* getGeomTypeString(int enumVal) {
	return GeomTypeStrings[enumVal];
	}

@alliecrevier ask:

[nitpick] Since T const& instead of const T& is the style used throughout this code file, it might make sense to use this style for pointers as well. So instead of const char* and static const char* you would use char const* and static char const* respectively. It looks weird to me but it's consistent!

[joto]

There is a huge controversy in the C++ community on this at the moment. This is now called "east const" vs. "const west". At the last big C++ conference (C++Now 2018) supporters of both sides gave out colored armbands to show support for on side or the other. Here is one blog post about this: http://slashslash.info/2018/02/a-foolish-consistency/

Anyway, consistency is good, so either one or the other.

[sssoleileraaa]

Looks like "const west" people won: https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#nl26-use-conventional-const-notation. But at least the "east const" people knew exactly which arm to wear the armband on.

(the right arm of course)

[joto]

@alliecrevier No, those core guidelines were written before this discussion blew up in the last few months and they are not the result of some kind of community consensus. The guidelines were critized in the discussion for using the wrong arguments. So this is still ongoing.

[sssoleileraaa]

@joto What do you think about using the C++ Core Guidelines except when we have reason to disagree?

As a way of making it easier to share and contribute to code in a consistent way, we can point people to the C++ Core Guidelines but tell them to check the Mapbox C++ Core-tech Guidelines for exceptions. Each project could even have their own set of guideline exceptions.

This could be a convenient way to organize our style guide. For instance, if we want to document that we use "east const" it could look something like:

NL.26-modified: Use east const notation

Reason
Even though conventional notation, aka const west notation, is more familiar to more programmers, we believe east const improves the readability of our code because it's simpler. The rule is that const should always be on the right of what it modifies.

Example

int const y = 9;  // recommended, const int
const int x = 7;  // not recommended, const int
int const * p = nullptr;  // recommended, pointer to const int
const int * p = nullptr;  // not recommended, pointer to const int
int * const  p = nullptr; // recommended (for both notations), const pointer to int
int const * const p = nullptr;  // recommended, const pointer to const int
const int * const p = nullptr;  // not recommended, const pointer to const int

Note
The recommended examples follow a simple rule so is easier to read and teach.

[joto]

Using the C++ Core Guidelines as a base and building on it is a good idea. Most of the Core Guidelines are uncontroversial and really good even if they are somewhat geared towards higher level code (pointer arithmetic is frowned upon for instance which is needed for low-level code).

But this disucssion has gone a "little" beyond what this issue was originally about and probably needs to be discussed somewhere else.