Mixing
Elegantly exclude part of an operation from the last iteration in a loop
Clash Royale CLAN TAG#URR8PPP
up vote
2
down vote
favorite
A well-formed comma-separated list has commas after all but the last element. This means that when dealing with these lists, the comma has to be dealt with on all but the last iteration. This is how I do things currently:
for (auto e = vec.cbegin(); e != vec.cend(); ++e)
// code that has to be executed for every element
std::cout << *e;
if (e != vec.cend() - 1)
// code that must not be executed for the last element
std::cout << ", ";
There are five instances of the above pattern in the project that I'm working on. I feel like there might be a more elegant way of doing this. An alternative that I've come up with is this:
// infinite loop if vec is empty
if (!vec.empty())
for (auto e = vec.cbegin(); e != vec.cend() - 1; ++e)
// code that has to be executed for every element
std::cout << *e;
// code that must not be executed for the last element
std::cout << ", ";
// code that has to be executed for every element
std::cout << vec.back();
This alternative is slightly faster because there isn't an if in the body but that's not a big deal. The if is true for all but the last iteration so the branch predictor will probably only miss on the last iteration. I also have to repeat code that has to be executed for every element which means that I need to hoist that into a function.
The first snippet is the best that I can come up with but I feel like this could be done better. Note that writing to std::cout is just an example.
c++ c++17
asked 2 hours ago
Kerndog73
442213
add a comment |Â
up vote
2
down vote
favorite
A well-formed comma-separated list has commas after all but the last element. This means that when dealing with these lists, the comma has to be dealt with on all but the last iteration. This is how I do things currently:
for (auto e = vec.cbegin(); e != vec.cend(); ++e)
// code that has to be executed for every element
std::cout << *e;
if (e != vec.cend() - 1)
// code that must not be executed for the last element
std::cout << ", ";
There are five instances of the above pattern in the project that I'm working on. I feel like there might be a more elegant way of doing this. An alternative that I've come up with is this:
// infinite loop if vec is empty
if (!vec.empty())
for (auto e = vec.cbegin(); e != vec.cend() - 1; ++e)
// code that has to be executed for every element
std::cout << *e;
// code that must not be executed for the last element
std::cout << ", ";
// code that has to be executed for every element
std::cout << vec.back();
This alternative is slightly faster because there isn't an if in the body but that's not a big deal. The if is true for all but the last iteration so the branch predictor will probably only miss on the last iteration. I also have to repeat code that has to be executed for every element which means that I need to hoist that into a function.
The first snippet is the best that I can come up with but I feel like this could be done better. Note that writing to std::cout is just an example.
c++ c++17
asked 2 hours ago
Kerndog73
442213
add a comment |Â
up vote
2
down vote
favorite
up vote
2
down vote
favorite
A well-formed comma-separated list has commas after all but the last element. This means that when dealing with these lists, the comma has to be dealt with on all but the last iteration. This is how I do things currently:
for (auto e = vec.cbegin(); e != vec.cend(); ++e)
// code that has to be executed for every element
std::cout << *e;
if (e != vec.cend() - 1)
// code that must not be executed for the last element
std::cout << ", ";
There are five instances of the above pattern in the project that I'm working on. I feel like there might be a more elegant way of doing this. An alternative that I've come up with is this:
// infinite loop if vec is empty
if (!vec.empty())
for (auto e = vec.cbegin(); e != vec.cend() - 1; ++e)
// code that has to be executed for every element
std::cout << *e;
// code that must not be executed for the last element
std::cout << ", ";
// code that has to be executed for every element
std::cout << vec.back();
This alternative is slightly faster because there isn't an if in the body but that's not a big deal. The if is true for all but the last iteration so the branch predictor will probably only miss on the last iteration. I also have to repeat code that has to be executed for every element which means that I need to hoist that into a function.
The first snippet is the best that I can come up with but I feel like this could be done better. Note that writing to std::cout is just an example.
c++ c++17
asked 2 hours ago
Kerndog73
442213
A well-formed comma-separated list has commas after all but the last element. This means that when dealing with these lists, the comma has to be dealt with on all but the last iteration. This is how I do things currently:
for (auto e = vec.cbegin(); e != vec.cend(); ++e)
// code that has to be executed for every element
std::cout << *e;
if (e != vec.cend() - 1)
// code that must not be executed for the last element
std::cout << ", ";
There are five instances of the above pattern in the project that I'm working on. I feel like there might be a more elegant way of doing this. An alternative that I've come up with is this:
// infinite loop if vec is empty
if (!vec.empty())
for (auto e = vec.cbegin(); e != vec.cend() - 1; ++e)
// code that has to be executed for every element
std::cout << *e;
// code that must not be executed for the last element
std::cout << ", ";
// code that has to be executed for every element
std::cout << vec.back();
This alternative is slightly faster because there isn't an if in the body but that's not a big deal. The if is true for all but the last iteration so the branch predictor will probably only miss on the last iteration. I also have to repeat code that has to be executed for every element which means that I need to hoist that into a function.
The first snippet is the best that I can come up with but I feel like this could be done better. Note that writing to std::cout is just an example.
c++ c++17
c++ c++17
asked 2 hours ago
Kerndog73
442213
asked 2 hours ago
Kerndog73
442213
asked 2 hours ago
Kerndog73
442213
asked 2 hours ago
Kerndog73
442213
asked 2 hours ago
Kerndog73
442213
442213
add a comment |Â
add a comment |Â
2 Answers
2
active
oldest
votes
up vote
3
down vote
I'm not sure that elegance should be your first concern here, because the algorithm should be very simple and the code short however you write them.
I/O operations will have a much greater impact on performance than the lay-out of your loop, so you shouldn't worry too much about speed either.
Generality, I believe, is paramount in this case. That's why I see vec.cend() - 1 as the main short-coming of your proposal, because it restricts your algorithm to at least bidirectional iterators, when your goal can be easily achieved for any kind of iterator.
Here's what I would consider better:
#include <iostream>
#include <vector>
template <typename Container, typename Separator> // any container is acceptable...
void pretty_print(const Container& c, const Separator& s)
std::cout << '';
auto it = std::begin(c); // ... even arrays
if (it != std::end(c)) do
std::cout << *it;
if (++it == std::end(c)) break; // that way you don't need to decrement the iterator
std::cout << s;
while (true);
std::cout << '';
answered 56 mins ago
papagaga
3,674221
I did mention that writing tostd::coutwas just an example but I do like that this works with forward iterators.
– Kerndog73
46 mins ago
add a comment |Â
up vote
2
down vote
If you are just doing a single-shot printing of a container, just print the first element and then print delim-value pairs. This reduces the iterator requirements to being equality comparable, dereferenceable, and incrementable.
if (!vec.empty()) // invert to return immediately if this is a function
auto first = vec.cbegin();
std::cout << *first;
while (vec.cend() != ++first)
std::cout << ", " << *first;
There is also a stateful approach that reduces the operation strength from branching on whether to print the delimiter to a pointer assignment.
auto actual_delim = ", ";
auto delim = "";
for (const auto& elem : vec)
std::cout << delim << elem;
delim = actual_delim;
See infix_iterator where this stateful approach is used.
answered 12 mins ago
Snowhawk
4,78911027
add a comment |Â
2 Answers
2
active
oldest
votes
2 Answers
2
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
3
down vote
I'm not sure that elegance should be your first concern here, because the algorithm should be very simple and the code short however you write them.
I/O operations will have a much greater impact on performance than the lay-out of your loop, so you shouldn't worry too much about speed either.
Generality, I believe, is paramount in this case. That's why I see vec.cend() - 1 as the main short-coming of your proposal, because it restricts your algorithm to at least bidirectional iterators, when your goal can be easily achieved for any kind of iterator.
Here's what I would consider better:
#include <iostream>
#include <vector>
template <typename Container, typename Separator> // any container is acceptable...
void pretty_print(const Container& c, const Separator& s)
std::cout << '';
auto it = std::begin(c); // ... even arrays
if (it != std::end(c)) do
std::cout << *it;
if (++it == std::end(c)) break; // that way you don't need to decrement the iterator
std::cout << s;
while (true);
std::cout << '';
answered 56 mins ago
papagaga
3,674221
I did mention that writing tostd::coutwas just an example but I do like that this works with forward iterators.
– Kerndog73
46 mins ago
add a comment |Â
up vote
3
down vote
I'm not sure that elegance should be your first concern here, because the algorithm should be very simple and the code short however you write them.
I/O operations will have a much greater impact on performance than the lay-out of your loop, so you shouldn't worry too much about speed either.
Generality, I believe, is paramount in this case. That's why I see vec.cend() - 1 as the main short-coming of your proposal, because it restricts your algorithm to at least bidirectional iterators, when your goal can be easily achieved for any kind of iterator.
Here's what I would consider better:
#include <iostream>
#include <vector>
template <typename Container, typename Separator> // any container is acceptable...
void pretty_print(const Container& c, const Separator& s)
std::cout << '';
auto it = std::begin(c); // ... even arrays
if (it != std::end(c)) do
std::cout << *it;
if (++it == std::end(c)) break; // that way you don't need to decrement the iterator
std::cout << s;
while (true);
std::cout << '';
answered 56 mins ago
papagaga
3,674221
I did mention that writing tostd::coutwas just an example but I do like that this works with forward iterators.
– Kerndog73
46 mins ago
add a comment |Â
up vote
3
down vote
up vote
3
down vote
I'm not sure that elegance should be your first concern here, because the algorithm should be very simple and the code short however you write them.
I/O operations will have a much greater impact on performance than the lay-out of your loop, so you shouldn't worry too much about speed either.
Generality, I believe, is paramount in this case. That's why I see vec.cend() - 1 as the main short-coming of your proposal, because it restricts your algorithm to at least bidirectional iterators, when your goal can be easily achieved for any kind of iterator.
Here's what I would consider better:
#include <iostream>
#include <vector>
template <typename Container, typename Separator> // any container is acceptable...
void pretty_print(const Container& c, const Separator& s)
std::cout << '';
auto it = std::begin(c); // ... even arrays
if (it != std::end(c)) do
std::cout << *it;
if (++it == std::end(c)) break; // that way you don't need to decrement the iterator
std::cout << s;
while (true);
std::cout << '';
answered 56 mins ago
papagaga
3,674221
I'm not sure that elegance should be your first concern here, because the algorithm should be very simple and the code short however you write them.
I/O operations will have a much greater impact on performance than the lay-out of your loop, so you shouldn't worry too much about speed either.
Generality, I believe, is paramount in this case. That's why I see vec.cend() - 1 as the main short-coming of your proposal, because it restricts your algorithm to at least bidirectional iterators, when your goal can be easily achieved for any kind of iterator.
Here's what I would consider better:
#include <iostream>
#include <vector>
template <typename Container, typename Separator> // any container is acceptable...
void pretty_print(const Container& c, const Separator& s)
std::cout << '';
auto it = std::begin(c); // ... even arrays
if (it != std::end(c)) do
std::cout << *it;
if (++it == std::end(c)) break; // that way you don't need to decrement the iterator
std::cout << s;
while (true);
std::cout << '';
answered 56 mins ago
papagaga
3,674221
answered 56 mins ago
papagaga
3,674221
answered 56 mins ago
papagaga
3,674221
answered 56 mins ago
papagaga
3,674221
3,674221
I did mention that writing tostd::coutwas just an example but I do like that this works with forward iterators.
– Kerndog73
46 mins ago
add a comment |Â
I did mention that writing tostd::coutwas just an example but I do like that this works with forward iterators.
– Kerndog73
46 mins ago
I did mention that writing to
std::cout was just an example but I do like that this works with forward iterators.– Kerndog73
46 mins ago
I did mention that writing to
std::cout was just an example but I do like that this works with forward iterators.– Kerndog73
46 mins ago
add a comment |Â
up vote
2
down vote
If you are just doing a single-shot printing of a container, just print the first element and then print delim-value pairs. This reduces the iterator requirements to being equality comparable, dereferenceable, and incrementable.
if (!vec.empty()) // invert to return immediately if this is a function
auto first = vec.cbegin();
std::cout << *first;
while (vec.cend() != ++first)
std::cout << ", " << *first;
There is also a stateful approach that reduces the operation strength from branching on whether to print the delimiter to a pointer assignment.
auto actual_delim = ", ";
auto delim = "";
for (const auto& elem : vec)
std::cout << delim << elem;
delim = actual_delim;
See infix_iterator where this stateful approach is used.
answered 12 mins ago
Snowhawk
4,78911027
add a comment |Â
up vote
2
down vote
If you are just doing a single-shot printing of a container, just print the first element and then print delim-value pairs. This reduces the iterator requirements to being equality comparable, dereferenceable, and incrementable.
if (!vec.empty()) // invert to return immediately if this is a function
auto first = vec.cbegin();
std::cout << *first;
while (vec.cend() != ++first)
std::cout << ", " << *first;
There is also a stateful approach that reduces the operation strength from branching on whether to print the delimiter to a pointer assignment.
auto actual_delim = ", ";
auto delim = "";
for (const auto& elem : vec)
std::cout << delim << elem;
delim = actual_delim;
See infix_iterator where this stateful approach is used.
answered 12 mins ago
Snowhawk
4,78911027
add a comment |Â
up vote
2
down vote
up vote
2
down vote
If you are just doing a single-shot printing of a container, just print the first element and then print delim-value pairs. This reduces the iterator requirements to being equality comparable, dereferenceable, and incrementable.
if (!vec.empty()) // invert to return immediately if this is a function
auto first = vec.cbegin();
std::cout << *first;
while (vec.cend() != ++first)
std::cout << ", " << *first;
There is also a stateful approach that reduces the operation strength from branching on whether to print the delimiter to a pointer assignment.
auto actual_delim = ", ";
auto delim = "";
for (const auto& elem : vec)
std::cout << delim << elem;
delim = actual_delim;
See infix_iterator where this stateful approach is used.
answered 12 mins ago
Snowhawk
4,78911027
If you are just doing a single-shot printing of a container, just print the first element and then print delim-value pairs. This reduces the iterator requirements to being equality comparable, dereferenceable, and incrementable.
if (!vec.empty()) // invert to return immediately if this is a function
auto first = vec.cbegin();
std::cout << *first;
while (vec.cend() != ++first)
std::cout << ", " << *first;
There is also a stateful approach that reduces the operation strength from branching on whether to print the delimiter to a pointer assignment.
auto actual_delim = ", ";
auto delim = "";
for (const auto& elem : vec)
std::cout << delim << elem;
delim = actual_delim;
See infix_iterator where this stateful approach is used.
answered 12 mins ago
Snowhawk
4,78911027
answered 12 mins ago
Snowhawk
4,78911027
answered 12 mins ago
Snowhawk
4,78911027
answered 12 mins ago
Snowhawk
4,78911027
4,78911027
add a comment |Â
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f207035%2felegantly-exclude-part-of-an-operation-from-the-last-iteration-in-a-loop%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
