If you know you’re operating on a std::vector, and that’s never going to change, there’s nothing fundamentally wrong with that form of for loop – it’s well understood, easy to read, and the compiler will usually do a good job of eliminating the potential cost of calling size() on every loop.

But using a for loop with iterators is more generic. It doesn’t require that your container have a size() operator at all, and doesn’t require that the container have index-based accessors at all (let alone efficient ones). So your code will be more generic if you go that route.

Other things to consider these days:

• The range-base for loop: compact syntax, very few requirements on the container type.
• Algorithms like for_each/find/fill/… combined with lambdas if the body of the loop lends itself to it for more functional-style C++.

Most of the time, both are about equally bad practice.

What you should normally try to do is figure out what the loop accomplishes, and either use an existing algorithm or else write a new algorithm to accomplish that end generically, then apply that algorithm to the collection/range at hand.

Just for example, let’s assume you were adding up the items in a vector:

int total = 0;

for (int i=0; i<v.size(); i++)
    total += v[i];

std::cout << "Total = " << total << "n";

Rewriting this to use iterators is essentially no improvement (and arguably detrimental):

int total = 0;

for (std::vector<int>::iterator p = v.begin(); p!= v.end(); ++p)
    total += *p;

std::cout << "Total = " << total << "n";

Using C++11, you can at least restore a little sanity:

int total = 0;

for (auto p : v)
    total += p;

std:cout << "Total = " << total << "n";

…but you don’t even need C++11 to to the job quite a bit better still:

std::cout << "Total = " 
          << std::accumulate(v.begin(), v.end(), 0)
          << "n";

When you find yourself thinking about writing a loop, changing the loop to use an iterator is rarely the right answer — the right answer is usually to eliminate the loop and replace it with an algorithm instead.

I wouldn’t say it’s a bad practice, I would say that using an iterator as the better practice.

Why? Because it’s more explicit. With a normal loop you aren’t necessarily using each element, you could be doing a related, but not directly connected operation. For instance a group chartering program might book X number of tickets because it has a group of size X, where the ticket assignment happens in a later stage.

Using the iterator says that you are going over the collection and doing something with each element. It conveys the intent of the program better.

It depends upon the implementation of the collection. Some collections do not have a fast and ready way to determine the number of elements they contain. For these collections, calling size() may require a complete iteration through the collection just to tell you how many there are. Such collections may not have an easy mechanism for random access either.

For example, a collection of “lines in a text file” would require parsing the entire text file just to find the number of lines in it. If you are just writing a loop to find one line in the file, you actually don’t care how many lines the file has, it is needed only to construct a loop in this way.

Such a collection can still be optimized to work well without random access ability. The hypothetical “lines in a text file” can easily give you each line, in order, quickly and efficiently. The Iterator object holds the current position, and knows how to get the next value, but nobody had to make an index of all the starting positions of all the lines in the file.

In summary, the iterator approach allows you to iterate such collections without having to know ahead of time how many elements there are, and without needing a fast random access method.