apache / arrow

Apache Arrow is a multi-language toolbox for accelerated data interchange and in-memory processing
https://arrow.apache.org/
Apache License 2.0
14.28k stars 3.47k forks source link

[C++] Improve clarity of MapBuilder API #25578

Open asfimport opened 4 years ago

asfimport commented 4 years ago

MapBuilder API is too much confusing. Everybody know Map is consist of key & value pairs. However, the API value_builder is actually the pairs builder. Instead, the item_builder, whose name is usually means the pairs builder, is the value builder.


  /// \brief Get builder to append items
  ///
  /// Appending an item with this builder should have been preceded
  /// by appending a key with key_builder().
  ArrayBuilder* item_builder() const { return item_builder_.get(); }

  /// \brief Get builder to add Map entries as struct values.
  ///
  /// This is used instead of key_builder()/item_builder() and allows
  /// the Map to be built as a list of struct values.
  ArrayBuilder* value_builder() const { return list_builder_->value_builder(); }

I know that we may want to keep align with the value_builder semantics with ListBuilder, but I don't think current naming is reasonable.

Reporter: Shuai Zhang / @hcoona

Note: This issue was originally created as ARROW-9510. Please see the migration documentation for further details.

asfimport commented 4 years ago

Antoine Pitrou / @pitrou: Well, the same terminology is used elsewhere. But I agree it's confusing. It's a side-effect of map being defined as a list of struct, rather than a type on its own.