0

I do have a question. I want to do a comparison of vector elements. If I write it with an old-style

for(int i = 0; i < vec.size()-1; i++){
if(vec[i] < vec[i+1])
...
}

But with new style I can't understand, how can do the same

for(auto a : vect)

is there any way to do the same job as above? Thank you

Numenorean
  • 5
  • 4
  • 1
    I guess you meant `vec.size() - 1`? No, not really, the above version is far better for that problem. – George Apr 05 '20 at 12:12
  • Yeah my mistake, it is out of range, I just wanted to demonstrate my problem. Thank you I will edit the question – Numenorean Apr 05 '20 at 12:17
  • There are some ways to get iterators in range-based for loop using boost so you could increment iterator to get next element https://stackoverflow.com/questions/22859348/have-range-based-for-loop-start-at-point-in-vector, but that seems pretty hard way to get what you want. I think it's better not to use "new style" here. – fas Apr 05 '20 at 12:28
  • 1
    You can write `for (const auto& a : vect) if ((&a != &vect.back()) && (a < *(&a+1))) ...`, but it's so ugly that I definitely wouldn't use it ;). Also, it is inefficient, since there is an additional comparison in each iteration. – Daniel Langr Apr 05 '20 at 12:31
  • Actually, `vec.size() - 1` results in `SIZE_MAX` if `vec.size() == 0`, which causes invalid memory access. – L. F. Apr 05 '20 at 13:15

2 Answers2

0

The correct style would be to use the appropriate algorithm for the task at hand. You should use the range-for loop only when you want to do a 

for (auto const & i : v) // std::for_each

or a 

for (auto & i : v) // std::transform

In this case, if you want to compare adjacent elements, you should use one of the std::adjacent... algorithms.

If you just want to find a position in the vector according to a predicate that compares adjacent elements, use

std::adjacent_find(std::begin(v), std::end(v));

If you want to transform a range according to a predicate that uses adjacent elements, use

std::adjacent_difference(std::begin(v), std::end(v), std::begin(v));

You should use a conventional for-loop only when there isn't an appropriate algorithm you can use (this happens quite rarely).

On a side note, your conventional for-loop has UB if the input range is empty. With -Wall the compiler will point out the problematic code. Prefer to write it like this

for(int i = 0; i < static_cast<int>(vec.size()) - 1; ++i){
  if(vec[i] < vec[i+1])
  // ...
}

Or like this, if you want to avoid casting

for(auto i = 0u; i + 1 < vec.size(); ++i){
  if(vec[i] < vec[i+1])
  // ...
}
cigien
  • 57,834
  • 11
  • 73
  • 112
0

With range-v3, you might do:

std::vector<int> v {1, 2, -3, 4, 5, -6};

for (auto p : ranges::view::zip(v, v | ranges::view::drop(1))
              | ranges::view::filter([](auto p){ return std::get<0>(p) < std::get<1>(p); })) {
    std::cout << std::get<0>(p) << " " << std::get<1>(p) << std::endl;   
}

Demo

Ranges from C++20 doesn't provide zip unfortunately.

Jarod42
  • 203,559
  • 14
  • 181
  • 302