Changing a variable out of scope of lambda expression

[FBF_Luis]

In my project I am trying to implement the Game of Life on a 40*40 grid. There is a button which starts/stop the animation and there are 3 numbers that will decide how the next generation of living cells is created: the repRule, the underRule and the overRule.
These three numbers are initialized at the beginning of mainwindow.cpp and I have a slider for each one of them as I want the user to be able to change them while running the application, the problem is that when I connect the sliders to a lambda function, the value of these integers do not change outside the scope of the lambda function. I have read some documentation on lambda functions, I have tried adding "&" before the variables like I did with a variable named "play" and I have tried to add the "mutable" keyword, but it still doesn't work.
Here is a minimal example that shows the problem I'm having: (I am debugging the values of the integers when using the sliders and when the animation is running)

mainwindow.h:



#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include <vector>
#include <iostream>

#include <QApplication>
#include <QPushButton>
#include <QGraphicsScene>
#include <QGraphicsItem>
#include <QGraphicsView>
#include <QTimer>


QT_BEGIN_NAMESPACE
namespace Ui
{
class MainWindow;
}
QT_END_NAMESPACE

using namespace std;

// global variables:
const int WIDTH = 660;
const int HEIGHT = 400;
const int SIZE = 10;
const int NCOL = HEIGHT/SIZE; //SIZE must be a divisor of HEIGHT

const QColor aliveCol = QColor(200,200,200);
const QColor deadCol = QColor(50,50,50);
const QColor textColor = Qt::white;

class Cell {
public:
int x;
int y;
bool alive;
vector<pair<int,int>> neigh;

Cell(int a = 0, int b = 0) {
x = a;
y = b;
alive = false;
vector<pair<int,int>> neigh;
}

void draw(QGraphicsScene *scene){
QGraphicsRectItem *cell1 = new QGraphicsRectItem(this->x*SIZE,this->y*SIZE,SIZE,SIZE);
if (this->alive) {
cell1->setBrush(QBrush(aliveCol));
}
else {
cell1->setBrush(QBrush(deadCol));
}
scene->addItem(cell1);
}

};

class MainWindow : public QMainWindow
{
Q_OBJECT

public:
MainWindow(QWidget *parent = nullptr);
~MainWindow();

void updateScene(Cell mat[NCOL][NCOL], QGraphicsScene *scene);

int countAlive(Cell c, Cell mat[NCOL][NCOL]);

void step(Cell mat[NCOL][NCOL], int rep, int under, int over);

private:
Ui::MainWindow *ui;
Cell mat[NCOL][NCOL] = {};
};
#endif // MAINWINDOW_H


mainwindow.cpp:


#include "mainwindow.h"
#include "ui_mainwindow.h"
#include <QtCore/QRandomGenerator>
#include <algorithm>
#include <QDebug>
#include <QSlider>

MainWindow::MainWindow(QWidget *parent)
: QMainWindow(parent)
, ui(new Ui::MainWindow)
{
ui->setupUi(this);
QGraphicsScene *scene = new QGraphicsScene(0, 0, WIDTH, HEIGHT+70);

//Rules:
int repRule = 3; //reproduction rule
int underRule = 1; //underpopulation rule
int overRule = 4; //overpopulation rule

//background:
scene->setBackgroundBrush(Qt::gray);

for (int i = 0; i <= NCOL; i++)
{
scene->addLine(SIZE*i,0,SIZE*i,HEIGHT);
scene->addLine(0,SIZE*i,HEIGHT,SIZE*i);
}

//adding the button:
QPushButton *button1;
button1 = new QPushButton();
button1->setGeometry(QRect(420, 20, 100, 30));
button1->setText("Start / Stop");
scene->addWidget(button1);

//adding sliders:
//reproduction slider
QSlider *repSlider;
repSlider = new QSlider(Qt::Horizontal);
repSlider->setGeometry(20,440,100,20);
repSlider->setRange(0,8);
repSlider->setValue(repRule);
scene->addWidget(repSlider);

QGraphicsTextItem *sl1Text = new QGraphicsTextItem("Reprodution:");
sl1Text->setPos(20,420);
sl1Text->setDefaultTextColor(textColor);
scene->addItem(sl1Text);

//underpopulation slider
QSlider *underPopSlider;
underPopSlider = new QSlider(Qt::Horizontal);
underPopSlider->setGeometry(140,440,100,20);
underPopSlider->setRange(0,8);
underPopSlider->setValue(underRule);
scene->addWidget(underPopSlider);

QGraphicsTextItem *sl2Text = new QGraphicsTextItem("Underpopulation:");
sl2Text->setPos(140,420);
sl2Text->setDefaultTextColor(textColor);
scene->addItem(sl2Text);

//overpopulation slider
QSlider *overPopSlider;
overPopSlider = new QSlider(Qt::Horizontal);
overPopSlider->setGeometry(260,440,100,20);
overPopSlider->setRange(0,8);
overPopSlider->setValue(overRule);
scene->addWidget(overPopSlider);

QGraphicsTextItem *sl3Text = new QGraphicsTextItem("Overpopulation:");
sl3Text->setPos(260,420);
sl3Text->setDefaultTextColor(textColor);
scene->addItem(sl3Text);

//building the cells matrix:
for (int i = 0; i<NCOL; i++) {
for (int j = 0; j<NCOL; j++) {
mat[i][j] = Cell(i,j);
}
}

//adding the neighbours:
for (int i = 0; i<NCOL; i++) {
for (int j = 0; j<NCOL; j++) {
mat[i][j].neigh.push_back(make_pair((i-1+NCOL)%NCOL,(j-1+NCOL)%NCOL));
mat[i][j].neigh.push_back(make_pair(i,(j-1+NCOL)%NCOL));
mat[i][j].neigh.push_back(make_pair((i+1)%NCOL,(j-1+NCOL)%NCOL));
mat[i][j].neigh.push_back(make_pair((i-1+NCOL)%NCOL,j));
mat[i][j].neigh.push_back(make_pair((i+1)%NCOL,j));
mat[i][j].neigh.push_back(make_pair((i-1+NCOL)%NCOL,(j+1)%NCOL));
mat[i][j].neigh.push_back(make_pair(i,(j+1)%NCOL));
mat[i][j].neigh.push_back(make_pair((i+1)%NCOL,(j+1)%NCOL)) ;
}
}

//have an initial layout for debugging
mat[20][19].alive = true;
mat[20][20].alive = true;
mat[20][21].alive = true;

bool play = false;

updateScene(mat, scene);

//make scene
QGraphicsView *view = new QGraphicsView(scene);
view->setFixedSize(670, 480);
view->show();

QTimer *_timer = new QTimer; //for the animation

//connect the button:
connect(button1, &QPushButton::clicked, this, [&play]()
{
play = !play;
}
);

//connect the sliders:
connect(repSlider, &QSlider::valueChanged, this, [&play, repSlider, &repRule](){
if(!play) {
repRule = repSlider->value();
qDebug() << repRule;
}
});

connect(underPopSlider, &QSlider::valueChanged, this, [&play, underPopSlider, &underRule](){
if(!play) {
underRule = underPopSlider->value();
qDebug() << underRule;
}
});

connect(overPopSlider, &QSlider::valueChanged, scene, [&play, overPopSlider, &overRule](){
if(!play) {
overRule = overPopSlider->value();
qDebug() << overRule;
}
});

connect(_timer, &QTimer::timeout, scene, [this, &play, scene, repRule, underRule, overRule]()
{
if (play) {
step(mat, repRule, underRule, overRule);
updateScene(mat, scene);
}
}
);

_timer->start(200);
}

MainWindow::~MainWindow()
{
delete ui;
}

void MainWindow::updateScene(Cell mat[NCOL][NCOL], QGraphicsScene *scene)
{
for (int i = 0; i<NCOL; i++) {
for (int j = 0; j<NCOL; j++) {
mat[i][j].draw(scene);
}
}
}

int MainWindow::countAlive(Cell c, Cell mat[NCOL][NCOL]) //auxiliary function for the step function
{
int r = 0;
for (pair<int,int> p : c.neigh) {
if (mat[get<0>(p)][get<1>(p)].alive) {r++;}
}
return r;
}

void MainWindow::step(Cell mat[NCOL][NCOL], int repN, int underN, int overN) //animation step
{
int aliveCount;
vector<pair<int,int>> living = {};
for (int i = 0; i < NCOL; i++) {
for (int j = 0; j < NCOL; j++) {
aliveCount = countAlive(mat[i][j],mat);
//rules:
if(!mat[i][j].alive){
if(aliveCount == repN) {
living.push_back(make_pair(i,j));
}
}
else if (aliveCount > underN && aliveCount < overN) {
living.push_back(make_pair(i,j));
}
}
}
for (int i = 0; i < NCOL; i++) {
for (int j = 0; j < NCOL; j++) {
mat[i][j].alive = std::find(living.begin(), living.end(), make_pair(i,j)) != living.end();
}
}
qDebug() << repN << underN << overN;
}


I will also take this opportunity to say that the animation runs slower with the time so there is probably a memory leak somewhere which I can't catch, so I'd appreciate is someone could help me with that aswell :)

[d_stranz]

//Rules:
int repRule = 3; //reproduction rule
int underRule = 1; //underpopulation rule
int overRule = 4; //overpopulation rule

bool play = false;

Of course, you understand that these are all local variables, defined only in the scope of the MainWindow constructor, and as soon as the constructor exits, the variables go away, right? So it doesn't matter how you capture them in the lambda, they simply don't exist once the constructor exits. I am fuzzy on this part, but I suspect that once the variables go out of scope, so do the connections to the timer and slider signals.

If you want to be able to access them outside the scope of the constructor, then they need to be member variables of the MainWindow class.

I don't even know if your lambda statements are correct for that matter. Instead of trying to be so cutting edge, you could easily turn all of the lambdas into actual slots in MainWindow, in which case you would have immediately discovered the problem with your variables and their scope. So why don't you do that first until you get everything running, then try the more advanced way of doing it.

For example, I have no idea what these are supposed to do, using QGraphicsScene as the connection target for the signals::



connect(overPopSlider, &QSlider::valueChanged, scene, [&play, overPopSlider, &overRule](){
if(!play) {
overRule = overPopSlider->value();
qDebug() << overRule;
}
});

connect(_timer, &QTimer::timeout, scene, [this, &play, scene, repRule, underRule, overRule]()
{
if (play) {
step(mat, repRule, underRule, overRule);
updateScene(mat, scene);
}
}
);


I don't see any obvious memory leak in your minimal example, so you have probably "minimaled" it out.

[FBF_Luis]

I am not sure this is what you meant, but I changed my code to this.

mainwindow.h:


#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>
#include <vector>
#include <iostream>

#include <QApplication>
#include <QPushButton>
#include <QGraphicsScene>
#include <QGraphicsItem>
#include <QGraphicsView>
#include <QTimer>
#include <QSlider>


QT_BEGIN_NAMESPACE
namespace Ui
{
class MainWindow;
}
QT_END_NAMESPACE

using namespace std;

// global variables:
const int WIDTH = 660;
const int HEIGHT = 400;
const int SIZE = 10;
const int NCOL = HEIGHT/SIZE; //SIZE must be a divisor of HEIGHT

const QColor aliveCol = QColor(200,200,200);
const QColor deadCol = QColor(50,50,50);
const QColor textColor = Qt::white;

class Cell {
public:
int x;
int y;
bool alive;
vector<pair<int,int>> neigh;

Cell(int a = 0, int b = 0) {
x = a;
y = b;
alive = false;
vector<pair<int,int>> neigh;
}

void draw(QGraphicsScene *scene){
QGraphicsRectItem *cell1 = new QGraphicsRectItem(this->x*SIZE,this->y*SIZE,SIZE,SIZE);
if (this->alive) {
cell1->setBrush(QBrush(aliveCol));
}
else {
cell1->setBrush(QBrush(deadCol));
}
scene->addItem(cell1);
}

};

class MainWindow : public QMainWindow
{
Q_OBJECT

public:
//Rules:
int repRule = 3; //reproduction rule
int underRule = 1; //underpopulation rule
int overRule = 4; //overpopulation rule

QSlider *repSlider;
QSlider *underPopSlider;
QSlider *overPopSlider;

QGraphicsTextItem *sl1Text = new QGraphicsTextItem("Reprodution:");
QGraphicsTextItem *sl2Text = new QGraphicsTextItem("Underpopulation:");
QGraphicsTextItem *sl3Text = new QGraphicsTextItem("Overpopulation:");

MainWindow(QWidget *parent = nullptr);
~MainWindow();

void updateScene(Cell mat[NCOL][NCOL], QGraphicsScene *scene);

int countAlive(Cell c, Cell mat[NCOL][NCOL]);

void step(Cell mat[NCOL][NCOL], int rep, int under, int over);

private:
Ui::MainWindow *ui;
Cell mat[NCOL][NCOL] = {};

public slots:
void updateRules();
};
#endif // MAINWINDOW_H


mainwindow.cpp


#include "mainwindow.h"
#include "ui_mainwindow.h"
#include <QtCore/QRandomGenerator>
#include <algorithm>
#include <QDebug>

MainWindow::MainWindow(QWidget *parent)
: QMainWindow(parent)
, ui(new Ui::MainWindow)
{
ui->setupUi(this);
QGraphicsScene *scene = new QGraphicsScene(0, 0, WIDTH, HEIGHT+70);

//background:
scene->setBackgroundBrush(Qt::gray);

for (int i = 0; i <= NCOL; i++)
{
scene->addLine(SIZE*i,0,SIZE*i,HEIGHT);
scene->addLine(0,SIZE*i,HEIGHT,SIZE*i);
}

//adding the button:
QPushButton *button1;
button1 = new QPushButton();
button1->setGeometry(QRect(420, 20, 100, 30));
button1->setText("Start / Stop");
scene->addWidget(button1);

//adding sliders:
//reproduction slider

repSlider = new QSlider(Qt::Horizontal);
repSlider->setGeometry(20,440,100,20);
repSlider->setRange(0,8);
repSlider->setValue(repRule);
scene->addWidget(repSlider);

sl1Text->setPos(20,420);
sl1Text->setDefaultTextColor(textColor);
scene->addItem(sl1Text);

//underpopulation slider
underPopSlider = new QSlider(Qt::Horizontal);
underPopSlider->setGeometry(140,440,100,20);
underPopSlider->setRange(0,8);
underPopSlider->setValue(underRule);
scene->addWidget(underPopSlider);

sl2Text->setPos(140,420);
sl2Text->setDefaultTextColor(textColor);
scene->addItem(sl2Text);

//overpopulation slider
overPopSlider = new QSlider(Qt::Horizontal);
overPopSlider->setGeometry(260,440,100,20);
overPopSlider->setRange(0,8);
overPopSlider->setValue(overRule);
scene->addWidget(overPopSlider);

sl3Text->setPos(260,420);
sl3Text->setDefaultTextColor(textColor);
scene->addItem(sl3Text);

//building the cells matrix:
for (int i = 0; i<NCOL; i++) {
for (int j = 0; j<NCOL; j++) {
mat[i][j] = Cell(i,j);
}
}

//adding the neighbours:
for (int i = 0; i<NCOL; i++) {
for (int j = 0; j<NCOL; j++) {
mat[i][j].neigh.push_back(make_pair((i-1+NCOL)%NCOL,(j-1+NCOL)%NCOL));
mat[i][j].neigh.push_back(make_pair(i,(j-1+NCOL)%NCOL));
mat[i][j].neigh.push_back(make_pair((i+1)%NCOL,(j-1+NCOL)%NCOL));
mat[i][j].neigh.push_back(make_pair((i-1+NCOL)%NCOL,j));
mat[i][j].neigh.push_back(make_pair((i+1)%NCOL,j));
mat[i][j].neigh.push_back(make_pair((i-1+NCOL)%NCOL,(j+1)%NCOL));
mat[i][j].neigh.push_back(make_pair(i,(j+1)%NCOL));
mat[i][j].neigh.push_back(make_pair((i+1)%NCOL,(j+1)%NCOL)) ;
}
}

//have an initial layout for debugging
mat[20][19].alive = true;
mat[20][20].alive = true;
mat[20][21].alive = true;

bool play = false;

updateScene(mat, scene);

//make scene
QGraphicsView *view = new QGraphicsView(scene);
view->setFixedSize(670, 480);
view->show();

QTimer *_timer = new QTimer; //for the animation

//connect the button:
connect(button1, &QPushButton::clicked, this, [&play]()
{
play = !play;
}
);

connect(repSlider, &QSlider::valueChanged, this, &MainWindow::updateRules);

connect(underPopSlider, &QSlider::valueChanged, this, &MainWindow::updateRules);

connect(overPopSlider, &QSlider::valueChanged, this, &MainWindow::updateRules);

connect(_timer, &QTimer::timeout, scene, [this, &play, scene]()
{
if (play) {
step(mat, repRule, underRule, overRule);
updateScene(mat, scene);
}
}
);

_timer->start(200);
}

MainWindow::~MainWindow()
{
delete ui;
}

void MainWindow::updateScene(Cell mat[NCOL][NCOL], QGraphicsScene *scene)
{
for (int i = 0; i<NCOL; i++) {
for (int j = 0; j<NCOL; j++) {
mat[i][j].draw(scene);
}
}
}

int MainWindow::countAlive(Cell c, Cell mat[NCOL][NCOL]) //auxiliary function for the step function
{
int r = 0;
for (pair<int,int> p : c.neigh) {
if (mat[get<0>(p)][get<1>(p)].alive) {r++;}
}
return r;
}

void MainWindow::step(Cell mat[NCOL][NCOL], int repN, int underN, int overN) //animation step
{
int aliveCount;
vector<pair<int,int>> living = {};
for (int i = 0; i < NCOL; i++) {
for (int j = 0; j < NCOL; j++) {
aliveCount = countAlive(mat[i][j],mat);
//rules:
if(!mat[i][j].alive){
if(aliveCount == repN) {
living.push_back(make_pair(i,j));
}
}
else if (aliveCount > underN && aliveCount < overN) {
living.push_back(make_pair(i,j));
}
}
}
for (int i = 0; i < NCOL; i++) {
for (int j = 0; j < NCOL; j++) {
mat[i][j].alive = std::find(living.begin(), living.end(), make_pair(i,j)) != living.end();
}
}
}

void MainWindow::updateRules()
{
repRule = repSlider->value();
underRule = underPopSlider->value();
overRule = overPopSlider->value();
}


There is probably a lot that can still be improved regarding where things should go but at least now the sliders are working fine when I mess with their values. The one thing that threw me off was the fact that it was working perfectly for the "play" variable, unlike the sliders...

[d_stranz]

The one thing that threw me off was the fact that it was working perfectly for the "play" variable, unlike the sliders...

I think you somehow got lucky with this:



connect(_timer, &QTimer::timeout, scene, [this, &play, scene, repRule, underRule, overRule]()
{
if (play) {
step(mat, repRule, underRule, overRule);
updateScene(mat, scene);
}
}
);


(Which makes absolutely no sense, because you have made "scene" the target of the connect() statement instead of "this". You have effectively invented a new slot for QGraphicsScene that doesn't exist in reality).

You have captured the address of "play" in the lambda, and even though it has gone out of scope, the lambda expression is still able to read and write to it. I think you got lucky that your program didn't crash, but it is probably because all the memory used by the program gets allocated in the MainWindow constructor and the address used by this variable didn't get reused for something else once the constructor had exited. If you had used "play" instead of "&play" it would not work.

[FBF_Luis]

That code came from a question I asked on a forum a while ago. It did what I intended at the moment and then I just added stuff and never got any problems with it, so I just went whit it.
Would then say that I should make "play" and "scene" also members from mainwindow class and use a slot instead of a lambda function to connect to that timer?

[d_stranz]

Would then say that I should make "play" and "scene" also members from mainwindow class and use a slot instead of a lambda function to connect to that timer?

That's how I would do it. Makes it much easier to debug when it is a slot and not a lambda.