Skip to content

Commit

Permalink
Addressed review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
JohanMabille committed Mar 21, 2024
1 parent 6f97284 commit b0a6565
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 15 deletions.
1 change: 0 additions & 1 deletion include/sparrow/mp_utils.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,5 @@ namespace sparrow

template <class T, bool is_const>
using constify_t = typename constify<T, is_const>::type;

}
}
14 changes: 9 additions & 5 deletions include/sparrow/variable_size_binary_layout.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,14 @@ namespace sparrow
* @brief Layout for arrays containing values consisting of a variable number of bytes.
*
* This layout is used to retrieve data in an array of values of a variable number of bytes
* (typically string objects). Values are stored contiguously in a data buffer, a single
* value is retrieved via an additional offset buffer, where each element is the beginning
* of the corresponinding value in the data buffer.
* (typically string objects). Values are stored contiguously in a data buffer (for instance
* a buffer of char if values are strings), a single value is retrieved via an additional
* offset buffer, where each element is the beginning of the corresponding value in the data
* buffer.
*
* Example:
*
* Let's consider the array ['please', 'allow', 'me', 'to', 'introduce', 'myself'].
* Let's consider the array of string ['please', 'allow', 'me', 'to', 'introduce', 'myself'].
* The internal buffers will be:
* - offset: [0, 6, 11, 13, 15, 24, 30]
* - data: ['p','l','e','a','s','e','a','l','l','o','w','m','e','t','o','i','n','t','r','o','d','u','c','e','m','y','s','e','l','f']
Expand Down Expand Up @@ -234,7 +235,7 @@ namespace sparrow
: m_data(std::move(data))
{
assert(m_data.buffers.size() == 2u);
//TODO: templatize back and front in buffer
//TODO: templatize back and front in buffer and uncomment the following line
//assert(m_data.buffers[0].size() == 0u || m_data.buffers[0].back() == m_data.buffers[1].size());
}

Expand Down Expand Up @@ -289,18 +290,21 @@ namespace sparrow
template <class T, class R, class CR, class OT>
auto variable_size_binary_layout<T, R, CR, OT>::offset(size_type i) const -> const_offset_iterator
{
assert(!m_data.buffers.empty());
return m_data.buffers[0].template data<OT>() + m_data.offset + i;
}

template <class T, class R, class CR, class OT>
auto variable_size_binary_layout<T, R, CR, OT>::offset_end() const -> const_offset_iterator
{
assert(!m_data.buffers.empty());
return m_data.buffers[0].template data<OT>() + m_data.length;
}

template <class T, class R, class CR, class OT>
auto variable_size_binary_layout<T, R, CR, OT>::data(size_type i) const -> const_data_iterator
{
assert(!m_data.buffers.empty());
return m_data.buffers[1].template data<data_type>() + i;
}
}
Expand Down
16 changes: 7 additions & 9 deletions test/test_variable_size_binary_layout.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,21 +66,19 @@ namespace sparrow

std::int64_t* offset()
{
assert(!m_data.buffers.empty());
return m_data.buffers[0].data<std::int64_t>();
}

static_assert(std::same_as<layout_type::inner_value_type, std::string>);
static_assert(std::same_as<layout_type::inner_const_reference, std::string_view>);
using const_value_iterator = layout_type::const_value_iterator;
static_assert(std::same_as<const_value_iterator::value_type, std::string>);
static_assert(std::same_as<const_value_iterator::reference, std::string_view>);
};

TEST_SUITE("variable_size_binary_layout")
{
TEST_CASE_FIXTURE(vs_binary_fixture, "types")
{
static_assert(std::same_as<layout_type::inner_value_type, std::string>);
static_assert(std::same_as<layout_type::inner_const_reference, std::string_view>);
using const_value_iterator = layout_type::const_value_iterator;
static_assert(std::same_as<const_value_iterator::value_type, std::string>);
static_assert(std::same_as<const_value_iterator::reference, std::string_view>);
}

TEST_CASE_FIXTURE(vs_binary_fixture, "size")
{
layout_type l(m_data);
Expand Down

0 comments on commit b0a6565

Please sign in to comment.