patterncppMinor
Container adaptors
Viewed 0 times
adaptorscontainerstackoverflow
Problem
How to improve the functionality, exception safeness and other aspects of the following container adaptors?
Allows to write more generic code (not working for, say, `
#include
#include
#include
template >::value >
struct consumable_container;
template
struct consumable_container
{
consumable_container(container && _container) noexcept
: container_(std::forward(_container))
{ ; }
auto
begin() noexcept
{
return std::make_move_iterator(std::begin(container_));
}
auto
end() noexcept
{
return std::make_move_iterator(std::end(container_));
}
private :
container container_;
};
template
struct consumable_container
{
static_assert(!std::is_rvalue_reference::value);
consumable_container(container && _container) noexcept
: container_(std::forward(_container))
{ ; }
auto
begin() const noexcept
{
return std::cbegin(container_);
}
auto
end() const noexcept
{
return std::cend(container_);
}
private :
container container_;
};
template
consumable_container
move_if_not_const(container && _container) noexcept
{
return std::forward(_container);
}
// some generic code
#include
template
auto
transform_to_list(container && _container) noexcept
{
static_assert(std::is_reference::value || !std::is_const::value);
std::list::value_type > list_;
for (auto && value_ : move_if_not_const(std::forward(_container))) {
list_.push_back(std::forward(value_));
}
return list_;
}
// testing data type
#include
struct A
{
A() { std::cout
#include
#include
#include
int
main()
{
{
std::deque const deque_(1);
std::list ld_ = transform_to_list(deque_);
}
std::cout forward_list_;
forward_list_.push_front(A{});
std::list ls_ = transform_to_list(forward_list_);
}
std::cout vector_;
vector_.push_back(A{});
std::list lv_ = transform
Allows to write more generic code (not working for, say, `
):
``#include
#include
#include
template >::value >
struct consumable_container;
template
struct consumable_container
{
consumable_container(container && _container) noexcept
: container_(std::forward(_container))
{ ; }
auto
begin() noexcept
{
return std::make_move_iterator(std::begin(container_));
}
auto
end() noexcept
{
return std::make_move_iterator(std::end(container_));
}
private :
container container_;
};
template
struct consumable_container
{
static_assert(!std::is_rvalue_reference::value);
consumable_container(container && _container) noexcept
: container_(std::forward(_container))
{ ; }
auto
begin() const noexcept
{
return std::cbegin(container_);
}
auto
end() const noexcept
{
return std::cend(container_);
}
private :
container container_;
};
template
consumable_container
move_if_not_const(container && _container) noexcept
{
return std::forward(_container);
}
// some generic code
#include
template
auto
transform_to_list(container && _container) noexcept
{
static_assert(std::is_reference::value || !std::is_const::value);
std::list::value_type > list_;
for (auto && value_ : move_if_not_const(std::forward(_container))) {
list_.push_back(std::forward(value_));
}
return list_;
}
// testing data type
#include
struct A
{
A() { std::cout
#include
#include
#include
int
main()
{
{
std::deque const deque_(1);
std::list ld_ = transform_to_list(deque_);
}
std::cout forward_list_;
forward_list_.push_front(A{});
std::list ls_ = transform_to_list(forward_list_);
}
std::cout vector_;
vector_.push_back(A{});
std::list lv_ = transform
Solution
The point of the code is to reduce verbosity:
Lets examine that:
Reverse is already done:
And my favorite:
Code Review
I don't see anything wrong (or that needs improving) with your code. Its nice and clean (some comments may hep the less experienced read the code). But overall it looks fine.
Goal of Code
I don't agree that the code achieves its goals (reduced verbosity of user code). I also think it makes boilerplate code more likely in user code (and boilerplate is bad as it tends to be cut and paste rather than written).
Forcing your user to write a whole section of code, rather than simplifying the task of writing code is where you fail.
I also think you need to understand where C++ has been going over the last 6->8 years. This is towards the use of ranges (with iterators as the underlying model used as the building block for ranges).
Lets examine that:
std::list ls_ = transform_to_list(forward_list_);
// Using iterators.
std::list ls_(begin(forward_list_), end(forward_list_));
// Don't think its that verbose.
// Not only does it work with list but it also works with every other container.Reverse is already done:
for (int i : reverse(vector_)) {
std::cout << i << std::endl;
}
// Or we can use the standard library (or probably future extensions)
// Slightly longer but its properly namespaced so that's understandable.
for (auto i : boost::adaptors::reverse(vector_))
std::cout << i << std::endl;
}And my favorite:
if (!vector_.empty()) {
for (int i : head(vector_)) {
std::cout << i << ", ";
}
std::cout << vector_.back() << std::endl;
}
// Or with a specialized iterator.
// Which definitely seems shorter and less verbose.
std::copy(std::begin(vector_), std::end(vector_), PrefexOutputIterator(std::cout, ","));Code Review
I don't see anything wrong (or that needs improving) with your code. Its nice and clean (some comments may hep the less experienced read the code). But overall it looks fine.
Goal of Code
I don't agree that the code achieves its goals (reduced verbosity of user code). I also think it makes boilerplate code more likely in user code (and boilerplate is bad as it tends to be cut and paste rather than written).
if (!vector_.empty()) {
for (int i : head(vector_)) {
std::cout << i << ", ";
}
std::cout << vector_.back() << std::endl;
}Forcing your user to write a whole section of code, rather than simplifying the task of writing code is where you fail.
I also think you need to understand where C++ has been going over the last 6->8 years. This is towards the use of ranges (with iterators as the underlying model used as the building block for ranges).
Code Snippets
std::list< A > ls_ = transform_to_list(forward_list_);
// Using iterators.
std::list< A > ls_(begin(forward_list_), end(forward_list_));
// Don't think its that verbose.
// Not only does it work with list but it also works with every other container.for (int i : reverse(vector_)) {
std::cout << i << std::endl;
}
// Or we can use the standard library (or probably future extensions)
// Slightly longer but its properly namespaced so that's understandable.
for (auto i : boost::adaptors::reverse(vector_))
std::cout << i << std::endl;
}if (!vector_.empty()) {
for (int i : head(vector_)) {
std::cout << i << ", ";
}
std::cout << vector_.back() << std::endl;
}
// Or with a specialized iterator.
// Which definitely seems shorter and less verbose.
std::copy(std::begin(vector_), std::end(vector_), PrefexOutputIterator(std::cout, ","));if (!vector_.empty()) {
for (int i : head(vector_)) {
std::cout << i << ", ";
}
std::cout << vector_.back() << std::endl;
}Context
StackExchange Code Review Q#71611, answer score: 2
Revisions (0)
No revisions yet.