remaining style cleanup

cpp/src/arrow/builder.cc

@@ -105,7 +105,7 @@ void ArrayBuilder::UnsafeAppendToBitmap(const uint8_t* valid_bytes, int32_t leng
   }
   for (int32_t i = 0; i < length; ++i) {
     // TODO(emkornfield) Optimize for large values of length?
-    AppendToBitmap(valid_bytes[i] > 0);
+    UnsafeAppendToBitmap(valid_bytes[i] > 0);
   }
 }
 

cpp/src/arrow/ipc/adapter.cc

@@ -103,12 +103,12 @@ Status VisitArray(const Array* arr, std::vector<flatbuf::FieldNode>* field_nodes
     buffers->push_back(std::make_shared<Buffer>(nullptr, 0));
   }
 
-  const auto arr_type = arr->type().get();
+  const DataType* arr_type = arr->type().get();
   if (IsPrimitive(arr_type)) {
-    const PrimitiveArray* prim_arr = static_cast<const PrimitiveArray*>(arr);
+    const auto prim_arr = static_cast<const PrimitiveArray*>(arr);
     buffers->push_back(prim_arr->data());
   } else if (IsListType(arr_type)) {
-    const ListArray* list_arr = static_cast<const ListArray*>(arr);
+    const auto list_arr = static_cast<const ListArray*>(arr);
     buffers->push_back(list_arr->offset_buffer());
     RETURN_NOT_OK(VisitArray(
         list_arr->values().get(), field_nodes, buffers, max_recursion_depth - 1));
@@ -142,9 +142,7 @@ class RowBatchWriter {
       int64_t size = 0;
 
       // The buffer might be null if we are handling zero row lengths.
-      if (buffer) {
-        size = buffer->size();
-      }
+      if (buffer) { size = buffer->size(); }
       // TODO(wesm): We currently have no notion of shared memory page id's,
       // but we've included it in the metadata IDL for when we have it in the
       // future. Use page=0 for now
@@ -154,7 +152,7 @@ class RowBatchWriter {
       // may (in the future) associate integer page id's with physical memory
       // pages (according to whatever is the desired shared memory mechanism)
       buffer_meta_.push_back(flatbuf::Buffer(0, position + offset, size));
-      
+
       if (size > 0) {
         RETURN_NOT_OK(dst->Write(position + offset, buffer->data(), size));
         offset += size;

cpp/src/arrow/ipc/ipc-adapter-test.cc

@@ -119,8 +119,10 @@ Status MakeListRowBatch(std::shared_ptr<RowBatch>* out) {
   std::shared_ptr<Array> leaf_values, list_array, list_list_array, flat_array;
   const bool include_nulls = true;
   RETURN_NOT_OK(MakeRandomInt32Array(1000, include_nulls, pool, &leaf_values));
-  RETURN_NOT_OK(MakeRandomListArray(leaf_values, length, include_nulls, pool, &list_array));
-  RETURN_NOT_OK(MakeRandomListArray(list_array, length, include_nulls, pool, &list_list_array));
+  RETURN_NOT_OK(
+      MakeRandomListArray(leaf_values, length, include_nulls, pool, &list_array));
+  RETURN_NOT_OK(
+      MakeRandomListArray(list_array, length, include_nulls, pool, &list_list_array));
   RETURN_NOT_OK(MakeRandomInt32Array(length, include_nulls, pool, &flat_array));
   out->reset(new RowBatch(schema, length, {list_array, list_list_array, flat_array}));
   return Status::OK();
@@ -140,7 +142,8 @@ Status MakeZeroLengthRowBatch(std::shared_ptr<RowBatch>* out) {
   std::shared_ptr<Array> leaf_values, list_array, list_list_array, flat_array;
   RETURN_NOT_OK(MakeRandomInt32Array(0, include_nulls, pool, &leaf_values));
   RETURN_NOT_OK(MakeRandomListArray(leaf_values, 0, include_nulls, pool, &list_array));
-  RETURN_NOT_OK(MakeRandomListArray(list_array, 0, include_nulls, pool, &list_list_array));
+  RETURN_NOT_OK(
+      MakeRandomListArray(list_array, 0, include_nulls, pool, &list_list_array));
   RETURN_NOT_OK(MakeRandomInt32Array(0, include_nulls, pool, &flat_array));
   out->reset(new RowBatch(schema, length, {list_array, list_list_array, flat_array}));
   return Status::OK();
@@ -157,11 +160,12 @@ Status MakeNonNullRowBatch(std::shared_ptr<RowBatch>* out) {
   MemoryPool* pool = default_memory_pool();
   const int length = 200;
   std::shared_ptr<Array> leaf_values, list_array, list_list_array, flat_array;
-  
+
   RETURN_NOT_OK(MakeRandomInt32Array(1000, true, pool, &leaf_values));
-  bool include_nulls = false; 
+  bool include_nulls = false;
   RETURN_NOT_OK(MakeRandomListArray(leaf_values, 50, include_nulls, pool, &list_array));
-  RETURN_NOT_OK(MakeRandomListArray(list_array, 50, include_nulls, pool, &list_list_array));
+  RETURN_NOT_OK(
+      MakeRandomListArray(list_array, 50, include_nulls, pool, &list_list_array));
   RETURN_NOT_OK(MakeRandomInt32Array(0, include_nulls, pool, &flat_array));
   out->reset(new RowBatch(schema, length, {list_array, list_list_array, flat_array}));
   return Status::OK();
@@ -188,8 +192,8 @@ Status MakeDeeplyNestedList(std::shared_ptr<RowBatch>* out) {
 }
 
 INSTANTIATE_TEST_CASE_P(RoundTripTests, TestWriteRowBatch,
-    ::testing::Values(&MakeIntRowBatch, &MakeListRowBatch, &MakeNonNullRowBatch, &MakeZeroLengthRowBatch, 
-        &MakeDeeplyNestedList));
+    ::testing::Values(&MakeIntRowBatch, &MakeListRowBatch, &MakeNonNullRowBatch,
+                            &MakeZeroLengthRowBatch, &MakeDeeplyNestedList));
 
 class RecursionLimits : public ::testing::Test, public MemoryMapFixture {
  public:
@@ -205,7 +209,8 @@ class RecursionLimits : public ::testing::Test, public MemoryMapFixture {
     RETURN_NOT_OK(MakeRandomInt32Array(1000, include_nulls, pool_, &array));
     for (int i = 0; i < recursion_level; ++i) {
       type = std::static_pointer_cast<DataType>(std::make_shared<ListType>(type));
-      RETURN_NOT_OK(MakeRandomListArray(array, batch_length, include_nulls, pool_, &array));
+      RETURN_NOT_OK(
+          MakeRandomListArray(array, batch_length, include_nulls, pool_, &array));
     }
 
     auto f0 = std::make_shared<Field>("f0", type);
@@ -247,9 +252,5 @@ TEST_F(RecursionLimits, ReadLimit) {
   ASSERT_RAISES(Invalid, reader->GetRowBatch(schema, &batch_result));
 }
 
-// TODO(emkornfield) More tests
-// Test primitive and lists with zero elements
-// Tests lists and primitives with no nulls
-// String  type
 }  // namespace ipc
 }  // namespace arrow

cpp/src/arrow/type.h

@@ -116,7 +116,7 @@ struct DataType {
 
   bool Equals(const DataType* other) {
     // Call with a pointer so more friendly to subclasses
-    return this == other || ((other != nullptr) && (this->type == other->type));
+    return other && ((this == other) || (this->type == other->type));
   }
 
   bool Equals(const std::shared_ptr<DataType>& other) { return Equals(other.get()); }

cpp/src/arrow/types/list.cc

@@ -24,10 +24,9 @@ bool ListArray::EqualsExact(const ListArray& other) const {
   if (this == &other) { return true; }
   if (null_count_ != other.null_count_) { return false; }
 
-  bool equal_offsets = offset_buf_->Equals(*other.offset_buf_, (length_ + 1) * sizeof(int32_t));
-  if (!equal_offsets) {
-    return false;
-  }
+  bool equal_offsets =
+      offset_buf_->Equals(*other.offset_buf_, (length_ + 1) * sizeof(int32_t));
+  if (!equal_offsets) { return false; }
   bool equal_null_bitmap = true;
   if (null_count_ > 0) {
     equal_null_bitmap =
@@ -47,9 +46,7 @@ bool ListArray::Equals(const std::shared_ptr<Array>& arr) const {
 
 Status ListArray::Validate() const {
   if (length_ < 0) { return Status::Invalid("Length was negative"); }
-  if (!offset_buf_) {
-    return Status::Invalid("offset_buf_ was null");
-  }
+  if (!offset_buf_) { return Status::Invalid("offset_buf_ was null"); }
   if (offset_buf_->size() / sizeof(int32_t) < length_) {
     std::stringstream ss;
     ss << "offset buffer size (bytes): " << offset_buf_->size()

cpp/src/arrow/types/primitive.cc

@@ -57,9 +57,7 @@ bool PrimitiveArray::EqualsExact(const PrimitiveArray& other) const {
     }
     return true;
   } else {
-    if (length_ == 0 && other.length_ == 0) {
-      return true;
-    }
+    if (length_ == 0 && other.length_ == 0) { return true; }
     return data_->Equals(*other.data_, length_);
   }
 }