rspec/rules/S5566/cfamily/rule.adoc

== Why is this an issue?

``++for++``-loops are a very powerful and versatile tool that can be used for many purposes. This flexibility comes with drawbacks:

* It is very easy to make a small mistake when writing them,
* They are relatively verbose to write,
* They do not express the intent of the code, the reader has to look at loop details to understand what the loop does.

There are algorithms that encapsulate a ``++for++``-loop and give it some meaning (``++std::all_of++``, ``++std::count_if++``, ``++std::remove_if++``...). These algorithms are well tested, efficient, and explicit and therefore should be your first choice.


This rule detects loops that go through all consecutive elements of a sequence (eg: containers, objects with begin() and end() member functions), and deal only with the current element without side-effects on the rest of the sequence.


This rule suggests using one of the supported STL algorithm patterns corresponding to your {cpp} standard when a loop matches it. 

Currently, this rule supports:

* ``++std::all_of++`` (since {cpp}11) and ``++std::ranges::all_of++`` (since {cpp}20): returns ``++true++`` if all elements in the given range are matching the given predicate, ``++false++`` otherwise
* ``++std::none_of++`` (since {cpp}11) and ``++std::ranges::none_of++`` (since {cpp}20): returns ``++true++`` if no elements in the given range are matching the given predicate, ``++false++`` otherwise
* ``++std::any_of++`` (since {cpp}11) and ``++std::ranges::any_of++`` (since {cpp}20): returns ``++true++`` if at least one element in the given range is matching the given predicate, ``++false++`` otherwise
* ``++std::ranges::contains++`` (since {cpp}23): returns ``++true++`` if at least one element in the given range is equal to the given value, ``++false++`` otherwise

This rule suggests two options below when the loop doesn't match any of the supported STL algorithm patterns and you just want to iterate over all elements of a sequence:

* Range-based ``++for++``-loops, which were introduced in {cpp}11 and will run through all elements of a sequence
* ``++std::for_each++``, an algorithm that performs the same operation between two iterators (allowing more flexibility, for instance by using ``++reverse_iterator++``s, or with a variant that can loop in parallel on several elements at a time).


=== Noncompliant code example

[source,cpp]
----
#include <vector>
#include <iostream>

using namespace std;

bool asDesired(const int v);

bool areAllDesired(std::vector<int> values) {
  for (int val : values) { // Noncompliant, replace it by a call to std::all_of
    if (!asDesired(val)) {
      return false;
    }
  }
  return true;
}

bool containsDesired(std::vector<int> values, int desired) {
  for (int val : values) { // Noncompliant
    if (val == desired) {
      return true;
    }
  }
  return false;
}

int f(vector<int> &v) {

  for (auto it = v.begin(); it != v.end(); ++it) { // Noncompliant
    if (*it > 0) {
      cout << "Positive number : " << *it << endl;
    } else {
      cout << "Negative number : " << *it << endl;
    }
  }

  auto sum = 0;
  for (auto it = v.begin(); it != v.end(); ++it) { // Noncompliant
    sum += *it;
  }
  return sum;
}
----


=== Compliant solution

[source,cpp]
----
#include <vector>
#include <iostream>
#include <algorithm>

using namespace std;

bool asDesired(const int v);

bool areAllDesired(std::vector<int> values) {
  return std::ranges::all_of(values, asDesired);
  // Or, before C++20:
  return std::all_of(std::begin(values), std::end(values), asDesired);
}

bool containsDesiredCpp23(std::vector<int> values, int desired) {
  return std::ranges::contains(values, desired);
  // Or, before C++23:
  return std::any_of(std::begin(values), std::end(values), [desired](int val) { return val == desired; });
}

void displayNumber(int i) {
  if (i > 0) {
    cout << "Positive number : " << i << endl;
  } else {
    cout << "Negative number : " << i << endl;
  }
}

void f(vector<int> &v) {

  std::ranges::for_each(v, displayNumber);
  // Or, before C++20:
  std::for_each(v.begin(), v.end(), displayNumber);

  auto sum = 0;
  for (auto elt : v) {
    sum += elt;
  }
  return sum;
  // An even better way to write this would be:
  // return std::accumulate(v.begin(), v.end(), 0); 
}
----


== Resources


=== Documentation

* {cpp} reference - https://en.cppreference.com/w/cpp/algorithm/ranges/contains[`std::ranges::contains`]
* {cpp} reference - https://en.cppreference.com/w/cpp/algorithm/ranges/all_any_none_of[`std::ranges::all_of`, `std::ranges::any_of`, `std::ranges::none_of`] 
* {cpp} reference - https://en.cppreference.com/w/cpp/algorithm/ranges/for_each[`std::ranges::for_each`] 
* {cpp} reference - https://en.cppreference.com/w/cpp/algorithm/all_any_none_of[`std::all_of`, `std::any_of`, `std::none_of`]
* {cpp} reference - https://en.cppreference.com/w/cpp/algorithm/for_each[`std::for_each`]

=== External coding guidelines

* {cpp} Core Guidelines - https://github.com/isocpp/CppCoreGuidelines/blob/e49158a/CppCoreGuidelines.md#es71-prefer-a-range-for-statement-to-a-for-statement-when-there-is-a-choice[ES.71: Prefer a range-`for`-statement to a `for`-statement when there is a choice]
* {cpp} Core Guidelines - https://github.com/isocpp/CppCoreGuidelines/blob/e49158a/CppCoreGuidelines.md#p3-express-intent[P.3: Express intent]

ifdef::env-github,rspecator-view[]

'''
== Implementation Specification
(visible only on this page)

=== Message

Convert this loop into a range-based for loop


'''
== Comments And Links
(visible only on this page)

=== on 5 Nov 2019, 18:30:54 Loïc Joly wrote:
\[~amelie.renard]: Can you please review my changes?

endif::env-github,rspecator-view[]