-2

I have a for loop in the below code and I would like to implement it using std::for_each. I have implemented it. Could someone please tell me if that is the best way to do it using std::for_each? If not, could you please suggest the right one?

#include <vector>
#include <cstdint>
#include <string>
#include <algorithm>
#include <iostream>
#include <sstream>

int main()
{
    std::vector<std::uint32_t> nums{3, 4, 2, 8, 15};
    std::stringstream list1;


    for (auto n : nums)
    {
        list1 << n<<",";
    }

    //Is this the right way to do using std::for_each so that above for loop can be done in 1 line??
    std::for_each(nums.begin(),nums.end(),[&list1](std::uint32_t n){ list1 << n << ","; });

}
user3852441
  • 162
  • 2
  • 10
  • You've overloaded `operator<` and `T` and even `const char*`? `std::vector` doesn't store commas, you know. This is not an interpreted language. – LogicStuff Feb 08 '16 at 16:59
  • This doesn't compile even when relevant headers are added as `std::vector` doesn't have operator `< – milleniumbug Feb 08 '16 at 16:59
  • Sorry for the mistake. I have updated the code. I hope it will compile now – user3852441 Feb 08 '16 at 17:06
  • Have you thought about, umm... compiling it yourself? – LogicStuff Feb 08 '16 at 17:09
  • Neither. This could be a [simple bind expression](http://ideone.com/m6pQgs). – Kerrek SB Feb 08 '16 at 17:16
  • Why `std::uint32_t`? Is there a requirement that this code only runs on systems with a 32-bit integral hardware type? Use `unsigned long`; it exists on all systems and is required to be at least 32 bits. Or, if you **really, really** like the explicitly sized types, use `std::uint_least32_t` or `std::uint_fast32_t`, both of which are required to exist on all platforms. – Pete Becker Feb 08 '16 at 17:31

4 Answers4

3

Yes, your use of for_each is a reasonable analog of the preceding loop.

I feel obliged to point out, however, that I find for_each probably the least useful algorithm in the library. From what I've seen, using it generally indicates that you're still basically thinking in terms of loops, and just changing the syntax you use for those loops. I also think that range-based for loops have probably eliminated at least 90% of the (already few) legitimate uses there used to be for for_each.

In this case, your code is really imitating using std::copy with an std::ostream_iterator:

std::copy(nums.begin(), nums.end(), 
          std::ostream_iterator<std::uint32_t>(std::cout, ","));

Even this, however, is clumsy enough that I think it's open to question whether it's really an improvement over a range-based for loop.

Jerry Coffin
  • 437,173
  • 71
  • 570
  • 1,035
1

Why don't you just test it out?

auto vs std::for_each

As you can see the assembly output is the same for both. It just doesn't make any difference for your example.

drescherjm
  • 8,907
  • 5
  • 42
  • 60
Simon Kraemer
  • 5,013
  • 1
  • 16
  • 43
  • Thanks for your answer. I tested it. I get the result. I wanted to know if there is a better way to do using std::for_each. – user3852441 Feb 08 '16 at 17:12
0

If you want to copy the data from one thing to another you can use std::copy

int main()
{
    std::vector<std::uint32_t> nums{3, 4, 2, 8, 15};
    std::stringstream list1;

    std::copy(nums.begin(), nums.end(),std::ostream_iterator<std::uint32_t>(list1,","));
    std::cout << list1.str();
}

Live Example

This will end the stream with a , but that is the same thing you get in you code.

If you do not want this then you should look at Pretty-print C++ STL containers

Community
  • 1
  • 1
NathanOliver
  • 150,499
  • 26
  • 240
  • 331
-4

Yes, that is right. There is no need for the reference in (std::uint32_t &n) if you had performance as the motive.

Vincent Savard
  • 30,767
  • 10
  • 61
  • 70
Milica
  • 1
  • 1