Like Antoine, I am not sure how comfortable I am with this the way it is now. On one hand I see the benefits in the reduction of boilerplate. On the other, making the code more "magical" through meta constructs likely makes it less accessible to contributors. It also makes it so that IDEs (Clion, VSCode, Visual Studio, or anything using clang-powered LSP) won't be able to autocomplete enums (please correct me if this is not true) — this alone to me is reason enough not to make these changes (particularly the new way of writing switch / if statements).
On Thu, Jul 22, 2021 at 1:49 PM Felipe Aramburu <fel...@blazingdb.com> wrote: > > It feels like not using regular enums would be optimizing more for the > producers of the library than the users that will be consuming it. It > could definitely reduce the burden of writing some boiler plate code but > this means that everyone that wants to use Arrow has another concept they > have to learn about in order to get started. > > ~felipe > > > > On Thu, Jul 22, 2021 at 11:52 AM Antoine Pitrou <anto...@python.org> wrote: > > > > > As I said on the PR, I'm worried that it will raise eyebrows or even > > make people defiant about using our C++ APIs. Basically, users of a > > language are accustomed to certain idioms and often wary of software > > packages that redefine their own variants of core language > > functionality. Even I find it slightly unsettling to write switch/case > > statements based on pointer values of literal strings... > > > > This is a social problem (on the technical front, the change seems ok, > > and I trust Benjamin that the underlying infrastructure is sound). > > > > This is why I would favor a mechanism where the reflection abilities are > > defined "on the side" (using e.g. a EnumTraits<...>), while the public > > C++ APIs still use normal enums. > > > > Regards > > > > Antoine. > > > > > > Le 22/07/2021 à 16:50, Benjamin Kietzman a écrit : > > > Enumerations are frequently useful constructs, but carry some overhead > > > when used with bindings. Specifically mapping the integral values to > > string > > > representations and back and validation of both integral values and > > strings > > > requires boilerplate, frequently replicated across multiple languages. > > > > > > https://github.com/apache/arrow/pull/10712 would replace some of the > > enums > > > defined in arrow/compute/api* with a metaprogramming construct which > > > includes validation, stringification, and parsing from string. > > > > > > The question at hand is: is it too confusing for users of the C++ API to > > > replace familiar enums with anything else? > > > > > > *** > > > > > > For example, one replacement from that PR is > > > > > > /// - `emit_null`: a null in any input results in a null in the > > output. > > > /// - `skip`: nulls in inputs are skipped. > > > /// - `replace`: nulls in inputs are replaced with the replacement > > > string. > > > struct NullHandlingBehavior : public EnumType<NullHandlingBehavior> > > { > > > using EnumType::EnumType; > > > static constexpr EnumStrings<3> values() { return {"emit_null", > > > "skip", "replace"}; } > > > static constexpr const char* name() { return > > "NullHandlingBehavior"; } > > > }; > > > > > > which replaces (not shown: deleted boilerplate for stringification, > > > validation, etc in Python and C++): > > > > > > enum NullHandlingBehavior { > > > /// A null in any input results in a null in the output. > > > EMIT_NULL, > > > /// Nulls in inputs are skipped. > > > SKIP, > > > /// Nulls in inputs are replaced with the replacement string. > > > REPLACE, > > > }; > > > > > > Values from an EnumType are constructed at compile time from a string, > > > so for example we don't lose the ability to write an expressive switch > > over > > > their values: > > > > > > switch (*null_handling) { > > > case *NullHandlingBehavior("emit_null"): > > > // ... > > > } > > > > >
