westonpace commented on code in PR #13069:
URL: https://github.com/apache/arrow/pull/13069#discussion_r867032904
##########
cpp/src/arrow/compute/exec/options.h:
##########
@@ -232,10 +232,13 @@ class ARROW_EXPORT SinkNodeConsumer {
/// \brief Add a sink node which consumes data within the exec plan run
class ARROW_EXPORT ConsumingSinkNodeOptions : public ExecNodeOptions {
public:
- explicit ConsumingSinkNodeOptions(std::shared_ptr<SinkNodeConsumer> consumer)
- : consumer(std::move(consumer)) {}
+ explicit ConsumingSinkNodeOptions(std::shared_ptr<SinkNodeConsumer> consumer,
+ std::vector<std::string> names = {})
+ : consumer(std::move(consumer)), names(std::move(names)) {}
std::shared_ptr<SinkNodeConsumer> consumer;
+ /// \brief Names to rename the sink's schema fields to
+ std::vector<std::string> names;
Review Comment:
In this case I think the naming scheme is correct. There are two categories
of objects we deal with, passive data objects and more complex stateful
objects. The google style guide likes to use structs for the former and
classes for the latter and so it describes this as [structs vs.
classes](https://google.github.io/styleguide/cppguide.html#Structs_vs._Classes).
In Arrow we are pretty inconsistent about applying `struct` and `class`.
Many of our "options" objects are `class` when they should be `struct` for
example. However, we are pretty consistent about the naming. For a passive
data object all properties should be public and [should not have trailing
underscores](https://google.github.io/styleguide/cppguide.html#Variable_Names).
Options objects should always be passive data objects. Things that travel
back and forth across library boundaries should generally be either passive
data objects or pure virtual interfaces when possible. So I do not think a
change is needed here. This options object should be a passive data object and
it is following the correct rules for a passive data object.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
