search

apache / arrow-nanoarrow

Helpers for Arrow C Data & Arrow C Stream interfaces
https://arrow.apache.org/nanoarrow
Apache License 2.0
149 stars 34 forks source link

refactor: Use ArrowStringView C++ literal in tests #528

Closed paleolimbot closed 1 week ago

paleolimbot commented 2 weeks ago

In https://github.com/apache/arrow-nanoarrow/pull/404, the _v literal was added, making it much more compact to define an ArrowStringView and compare its equality to something else.

This PR refactors tests that used EXPECT/ASSERT_EQ() to be more compact, and replaces ArrowCharView() with the string literal.

From reading https://en.cppreference.com/w/cpp/string/basic_string_view/operator%22%22sv and https://json.nlohmann.me/api/macros/json_use_global_udls/#notes it seems like constraining the user-defined literal to a namespace is the "modern" way to do this...I am not sure that "_sv" is the best choice (maybe too close to "sv"?), but it did seem more descriptive than "_v for those reading the tests without knowing that these literals are defined.

WillAyd commented 1 week ago

Very cool! lgtm - only thing I could suggest is _asv versus _sv to disambiguate from the stdlib a little more, but not a strong opinion

paleolimbot commented 1 week ago

Thank you for the review!

only thing I could suggest is _asv versus _sv to disambiguate from the stdlib a little more

Great idea!