Как правильно применить функцию ко всем элементам двумерного вектора?

Question

[Fortunato28]

std::for_each(matrix.begin(), matrix.end(), [&](vector<Cell> cellLine)
    {
        std::for_each(cellLine.begin(), cellLine.end(), [&](Cell cell)
        {
            cell.showBody();
        });
    });

Приведёный выше код компилируется и работает, обходит каждый элемент матрицы, как будто вызывает нужный метод каждой клетки, но при этом сами элементы двумерного вектора не меняются, хотя должны. Подозреваю, что где-то не понимаю чего-то в лямбдах. В чём тту дело?

Comments

[res2001]

У вас кроме cell.showBody() никаких действий с элементами не происходит.

[Евгений Шатунов]

Fortunato28

но при этом сами элементы двумерного вектора не меняются, хотя должны.

А кто это, собственно, сказал, что элементы что-то должны, и почему мы должны верить этому утверждению? :)
Пока у тебя в коде видно только ошибку на ошибке, поэтому я не верю что этот код работает и вызывает что-либо нужное. Наоборот, ты говоришь неправду, этот код не работает, не обходит каждый элемент матрицы и не вызывает нужный метод.

И неправду ты говоришь потому что...

... (vector<Cell> cellLine) ...
... (Cell cell) ...

Такие дела... :)

[res2001]

Евгений Шатунов, Тут вроде все нормально - 2 вложенных цикла - обход двумерного контейнера.
Но элементы действительно не меняются, т.к. в коде нет никаких замен элементов.

[Евгений Шатунов]

res2001 , и в лямбды идет передача по значению. Яж показал этот момент.
А что у нас получается, когда вектор в функцию передается по значению?

[res2001]

Евгений Шатунов, верно, спс.

[Евгений Шатунов]

Fortunato28 , "ошибка на ошибке" - это:

• мешанина из отсутствия и наличия std;
  
• использование std::vector<std::vector<Cell>> вместо std::vector<Cell>;
  
• использование std::for_each там, где достаточно простого ranged-for;
  
• использование .begin()/.end() вместо более подходящих std::begin()/std::end() при содействии ADL;
  
• наличие замыкания на весь контекст по ссылке;
  
• неразбериха с мутабильностью объектов типа Cell;
  
• (вишенка на торте) передача параметров по значению, что приводит к фактическому копированию объектов.
  

Именно из за передачи по значению у тебя что-то вызывается, но объекты изначальной матрицы не меняются. Все дело в том, что перед изменением они просто копируются, а после изменения копии сразу же уничтожаются.

[Fortunato28]

Евгений Шатунов, спасибо за замечания, хоть и достаточно саркастичные.

1. Мешанина отсутствия и наличия std разрешается using std::vector в заголовочнике и т.д.
   

2. использование std::vector> вместо std::vector

   - это ведь обход матрицы, и "внешний" её уровень - вектор, содержащий вектора класса Cell.
   
3. Спасибо за ranged-for, буду знать.
   

4. наличие замыкания на весь контекст по ссылке

   - Вот здесь, к сожалению, ничего не понял. Поясните, пожалуйста.
   
5. Объекты типа Cell должны были меняться при вызове метода showBody() (а точнее атрибуты этих объектов, вот что я имел в виду.) А не происходило этого из-за передачи в лямбду по значению.
   

[Евгений Шатунов]

Fortunato28 , не, мои замечания ни чуть не саркастичны. У тебя как, матрица может быть нерегулярной?
Один столбец имеет 2 элемента, а другой - 8? Явно нет, матрица у тебя регулярная, а значит разметить ты ее можешь единым полотном памяти, доступ к которому можно легко получать используя операцию умножения. Дополнительно ты себе скорость обхода сохранишь на приемлемом уровне, т.к. предсказатель кеша при поступательном итерировании всегда будет прав.

Вот здесь, к сожалению, ничего не понял. Поясните, пожалуйста.

Посмотри на документацию для лямбд.
В [] указывается замыкание. Замыкание [&] говорит, что лямбда автоматически захватывает любую переменную текущего контекста по ссылке, если она используется в теле лямбды. "Текущий контекст" - это всё выше лямбды по области видимости.
Иными словами, ты указал то, чем не пользуешься. Этот путь ведет к плохим последствиям. Возможно ты просто воспользовался инструментом, суть работы которого не понимаешь.

Мешанину и метания надо исключить. Код должен быть однозначным иметь единый стиль, иметь понятные имена идентификаторов и читаться как будто это описание договора. От using namespace std; лучше отказаться в сторону лучшего понимания написанного кода.

[Евгений Шатунов]

Fortunato28, и, это, поставь чтоли ребятам решения за ответы, не игнорируй правила тостера. :)

Answer 1

[Roman]

что то мудрено как то...

for(auto &cellLine:matrix)
{
    for(auto &cell:cellLine)
    {
        cell.showBody();
    }
}

foreach и лямбда-функции здесь явно лишние.
про передачу по ссылке уже написал cppjunkie

Comments

[Fortunato28]

Спасибо, всё понял.

Answer 2

[cppjunkie]

Не меняются скорее всего из-за того что аргумент в лямбду передаётся по значению, поэтому весь вектор копируется и изменяется уже его копия. По-моему если поменять с

vector<Cell> cellLine

на

vector<Cell> &cellLine

то должно заработать. То же самое надо сделать и во внутреннем цикле относительно параметра Cell.

Comments

[Fortunato28]

Да, вы правы, с передачей по ссылке всё работает так, как нужно. Спасибо.