Sieve of Eratosthenes in C++11

Please point out every problem you see with this code. Any fundamental flaws, naming conventions, design decisions, you name it.

#include 
#include 
#include 

class SieveHelper {
public:
    SieveHelper() : cur(2) {}

    bool operator()(unsigned int i) {
        return i != cur && i % cur == 0;
    }

    unsigned int cur;
};

void sieve(std::vector& arr) {
    SieveHelper helper;
    auto it = arr.begin();

    while (it != arr.end()) {
        helper.cur = *it;

        auto last = std::remove_if(it, arr.end(), helper);
        arr.erase(last, arr.end());

        it++;
    }
}

int main() {
    int n;
    std::cin >> n;

    std::vector arr(n);
    for (int i = 2; i <= n; i++)
        arr[i-2] = i;

    sieve(arr);

    std::for_each(arr.begin(), arr.end(), [](unsigned int i) {
        std::cout << i << " ";
    });
    return 0;
}

• I sort includes alphabetically, making it easier to keep track of which headers are included in long lists.

I would write:

#include 
#include 
#include 

In a long, unsorted list of includes it's hard to see if a header is included twice:

#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 
#include 

Imagine this problem increasing if you have a couple dozen includes!

• arr is very undescriptive, and in this case even misleading. This isn't an array, it's a std::vector. vec would be slightly better, but try to describe what the data structure contains or does rather than what kind of data structure it is. Even something as simple as numbers is better than vec (or arr), and you can probably improve that even further. Naming is very important to ensure readability.

• Consider using to control the size of numbers. This makes porting a whole lot easier.

• In C++, it's a good habit to prefer prefix increment- and decrement-operators, because they are usually more efficient for non-built-in types. It doesn't really matter for built-in types, but consistency is nice.

• Consider using using-declarations, i.e. using std::vector; and so on, to make the code more readable.

• I'd usually make SieveHelper::cur private and control access to it, but I presume you know that and did it this way on purpose. It's fine for a small program, but remember that programs grow.

• Use std::vector::at(), which is bounds-checked, whenever array indexing is not in the speed-critical path. The exception is when you're 110 % sure you can never, ever go out of bounds - but better safe than sorry!

• Consider making sieve() take and return an argument, rather than just manipulate an output argument. This is definitely a trade-off, so use your own judgement, but output parameters are generally less readable.

On the plus side, this is mostly good. Extra points for using built-in algorithms, lambda and other juicy C++11 stuff :)

StackExchange Code Review Q#7245, answer score: 6