Изменение возвращаемого лямбда-функцией значения

Question

Изменение возвращаемого лямбда-функцией значения

Рейтинг: 0Ответов: 1Опубликовано: 16.02.2023

Есть класс Field, который содержит вектор из экземпляров класса Cell. Нужно для экземпляров класса Cell менять значение поля Status функцией ChangeStatus. Изменять значение поля нужно для элементов с известными координатами X и Y, для чего используется лямбда-функция find_if.

Класс Cell:

class Cell {
    private:
        int X;
        int Y;
        CellStatus Status;
    public:
        Cell(): X(0), Y(0), Status(CellStatus::Empty) {};
        Cell(int newX, int newY, CellStatus newCS) : Cell() 
        {
            X = newX;
            Y = newY;
            Status = newCS;
        };
        ~Cell() {};
    
    
        int getX() {return X;}
        int getY() {return Y;}  
        CellStatus getStatus() {return Status;}
        void ChangeStatus(CellStatus newCS) 
        {
            Status = newCS;
        };
    
        Cell& operator= (Cell& otherCell) {
            Status = otherCell.getStatus();
            return *this;
        }
    };

Класс Field:

class Field
{
private:
    vector<Cell> cells;
public:
    Field(){
        //Создание элементов вектора cells - ячеек поля
        for (int i = 1; i < 9; i++) 
        {
            for (int j = 1; j < 9; j++) 
            {
                if((i==4 && j == i) || (i==5 && j==5)) cells.push_back(Cell(j, i, White));
                else if ((i == 4 && j == 5) || (i == 5 && j == 4)) cells.push_back(Cell(j, i, Black));
                else cells.push_back(Cell(j, i, Empty));
            }
        }
    };
    ~Field(){};

    vector <Cell> getField() { return cells; }

    Cell& getCellStatusByXY(int searchedX, int searchedY) //Узнать цвет ячейки по её координатам (для лямбда-функции)
    {
         auto foundC = find_if(cells.begin(), cells.end(), [&](Cell &c)
            {
                return (c.getX() == searchedX && c.getY() == searchedY);
            });
         return (*foundC);
    }

    void fillColorForArray(vector<Cell> cellsToSwap, CellStatus colorToSwap) //Заменить цвет ячеек, идентичных ячейкам из массива
    {
        for_each(cellsToSwap.begin(), cellsToSwap.end(), [&](Cell &cellNew) //Фишки, которым мы меняем цвет
            {
                auto foundCell = find_if(getField().begin(), getField().end(), [&](Cell cell) //Фишки, которые мы ищем
                    {
                        return (cell.getX() == cellNew.getX() && cell.getY() == cellNew.getY());
                    });
                
                (*foundCell).ChangeStatus(CellStatus::Possible);
            });
    }
};

Пример использования лямбда-функции есть в классе Field, в функции fillColorForArray. Проблема в том, что после выполнения функции меняется значение Status у ячейки foundCell, которую искали лямбда-функцией, а не элемент вектора cells класса Field.

Как это можно исправить? Поскольку в некоторых местах проекта впоследствии придётся таким образом использовать лямбда-функцию в лямбда-функции в лямбда-функции, есть вопрос: можно ли это сделать иначе?

c# lambda

Источник: Stack Overflow на русском

Answer 1

▲ 4Принят

У вас проблема не в лямбда-функции, а в том, что метод getField() возвращает копию вектора, хранящегося в class Field. А раз вы собираетесь его менять, то нужно вернуть ссылку. И название метода у вас неинформативно - по названию больше похоже что вы хотите вернуть ячейку. Назовите его как-то так: getAllCells() или getFieldCells()

// вместо
vector <Cell> getField() { return cells; }

// надо
vector<Cell>& getField() { return cells; }

Также у вас неинформативно название метода getCellStatusByXY(). Вы ведь возвращаете ссылку на ячейку. Ну и назовите getCellByXY(). И поле Status в классе Cell - дальше вы ведь пишете, что это цвет.
Ссылку на весь вектор нужно вернуть, если вы собираетесь изменять вектор за пределами класса, что нарушает инкапсуляцию данных. Тогда проще было бы сделать вектор в секции public. Поэтому правильнее вернуть ссылку не на весь вектор, а на нужный элемент. И для этого у вас уже есть метод!
Ещё могут быть проблемы вот в этом месте - здесь вернется две разных копии вектора, и пройти по элементам от начала первой копии до конца второй копии скорее всего закончится ошибкой. Ошибка возникнет если элемент не будет найден.

// поиск от начала одного вектора до конца другого вектора
find_if(getField().begin(), getField().end(), [&](Cell cell){} )

И зачем вы в одном методе напрямую обращаетесь к вектору cells, а в другом - через метод getField()? Обращайтесь также напрямую - ошибка уйдет.

find_if(cells.begin(), cells.end(), [&](Cell cell){} )

Ещё возможная ошибка в методе getCellStatusByXY(). Если ячейка с указанными координатами не будет найдена, то используемая внутри функция find_if() вернет итератор cells.end() - ссылка на фиктивный (несуществующий) элемент, следующий за последним в векторе. Сделать с ним что-то нельзя - только сравнивать итераторы. И похоже вы увлеклись стандартными функциями. Чем хуже реализация метода с циклом?

Cell& getCellStatusByXY(int searchedX, int searchedY) //Узнать цвет ячейки по её координатам (для лямбда-функции)
    {
         auto foundC = find_if(cells.begin(), cells.end(), [&](Cell &c){....});
         return (*foundC);
    }

Cell& getCellStatusByXY(int searchedX, int searchedY)
{
    for( auto &c : cells )
        if( c.X == searchedX && c.Y == searchedY) 
            return c;
    return cells.end(); // что-то в случае ошибки
}

В методе fillColorForArray() вы получаете копию вектора в качестве исходных данных. Этот вектор может быть большим. Для экономии памяти лучше передать по константной (чтобы случайно не изменить данные) ссылке - это аналог передачи копии.

void fillColorForArray(const vector<Cell>& cellsToSwap, CellStatus colorToSwap){}

Также у вас дублирование кода. Вложенная лямбда в методе fillColorForArray() - это поиск ячейки по координатам. А для этого у вас уже есть метод getCellStatusByXY(). Для удобства можно сделать его перегрузку

Cell& getCellStatusByXY(int searchedX, int searchedY)
Cell& getCellStatusByXY(const Cell& another) { getCellStatusByXY( another.getX(), another.getY() ); }

И тогда можно переписать без вложенной лямбды:

void fillColorForArray(vector<Cell> cellsToSwap, CellStatus colorToSwap) 
{
    for_each(cellsToSwap.begin(), cellsToSwap.end(), [&](const Cell &cellNew)
    {
        auto foundCell = getCellStatusByXY(cellNew);
        (*foundCell).ChangeStatus(CellStatus::Possible);
// или даже так - в стиле функционального программирования
//        getCellStatusByXY(cellNew).ChangeStatus(CellStatus::Possible);
    });
}

В вашем случае использование лямд не совсем оправданно, от них вообще можно избавиться. У вас они сравнивают ячейки одна по статусу, другая по координатам. Скорее всего такие сравнения будут нужны не единожды и поэтому их лучше оформить методами класса class Cell. А лямды пишутся для одноразового использования - ведь если этот код используется только в одном месте, то зачем перегружать интерфейс класса?

class Cell
{
    bool equalStatus(const Cell& c) const { return Status == c.GetStatus();}
    bool equalCoord(const Cell& c) const
    {
        return ( X == c.getX() && Y == c.getY() );
    }
}

Изменение возвращаемого лямбда-функцией значения

Ответы