Implementation of Conway's Game of Life



  • Hello,

    at first, I am not sure if this is the right category. Please move it if not.
    Also I am sorry for any English mistakes.

    Some time ago I wrote a implementation of Conway's well known Game of Life with a GUI. Now I improved it a little bit with the help of another (german) C++-forum, but I think there are still some design issues and/or bugs.
    So I want to ask you if you can help me to improve my code and my Qt skills.
    Someone in the other forum criticized my SeparateThread-class (not a nice name; maybe Worker-class is better) and my methode to use QThread. But he couldn't say how I can make this better and I have no other idea.
    If you find anything else or have some tips, please write it - I think I am open enough for any criticism.

    Thank you!

    Sorry, I forgot the link.
    Here


  • Moderators

    @SqYt There is one good article describing how to use multi-threading in Qt: https://mayaposch.wordpress.com/2011/11/01/how-to-really-truly-use-qthreads-the-full-explanation/



  • Thank you, I know this article.
    But I didn't sub-classed QThread.
    I have a Worker-class (here: SeparateThread) and a QThread-object. Im the ctor of MainWindow I move the object of SeparateThread in the QThread and connect a own signal to the public slot of SeparateThread. In this slot is a loop, which calculates the next generation and updates GraphicalWindow, which inherits from QGraphicsView. To leave the loop I use a reference inside the worker to an int in MainWindow.
    Each time I want to start the loop, I emit my signal. The QThread runs between the ctor of MainWindow and the dtor the whole time, because I need only one other thread for timing the update interval. Maybe a QTimer would help, but I think that it would be more complicated.


  • Moderators

    @SqYt It is hard to follow your description. Can you post relevant parts of the code?



  • Sorry, here is the relevant code:

    seperatethread.cpp:

    #include "seperatethread.h"
    #include <QApplication>
    #include <QMutex>
    
    SeperateThread::SeperateThread(GraphicalWindow* graphic, Settings* settings, Label *label, int &loops, QObject *parent) : QObject(parent), graphic(graphic), settings(settings), loops(loops)
    {
    	QObject::connect(this, SIGNAL(changedTimer(uint)), label, SLOT(changeTimer(uint)));
    }
    
    void SeperateThread::work()
    {
    	if(loops == -1)
    	{
    		while(loops != -2)
    		{
    			mutex.lock();
    			graphic->nextGraphicGen();
    			emit changedTimer(graphic->getGeneration());
    			graphic->scene->update(graphic->scene->sceneRect());
    			QThread::msleep(settings->getAutoplaySpeed());
    			mutex.unlock();
    		}
    	}
    	else if(loops == -3)
    	{
    		mutex.lock();
    		graphic->nextGraphicGen();
    		emit changedTimer(graphic->getGeneration());
    		graphic->scene->update(graphic->scene->sceneRect());
    		loops = 3;
    		mutex.unlock();
    	}
    	else
    	{
    		for(int i = 0; i < loops && loops > 0; ++i)
    		{
    			mutex.lock();
    			graphic->nextGraphicGen();
    			emit changedTimer(graphic->getGeneration());
    			graphic->scene->update(graphic->scene->sceneRect());
    			QThread::msleep(settings->getAutoplaySpeed());
    			mutex.unlock();
    		}
    	}
    }
    

    relevant part of mainwindow.cpp:

    MainWindow::MainWindow(QWidget* parent) : QMainWindow(parent)
    {
            // ...
    	secondThread = new QThread(this);
    	autoplayBreak = -3;
    	worker = new SeparateThread(graphic, settwin->settings(), generationTimer, autoplayBreak);
    	worker->moveToThread(secondThread);
    	QObject::connect(this, SIGNAL(startThread()), worker, SLOT(work()));
            secondThread->start();
    }
    

    Each time I want to execute the slot work() I use:

    emit startThread();
    

    To exit the thread I use:

    MainWindow::~MainWindow()
    {
    	pause();
    	secondThread->wait(5);
    	secondThread->exit(0);
    
    	// ...
    	delete secondThread;
    }
    

  • Moderators

    @SqYt said in Implementation of Conway's Game of Life:

    graphic

    this graphic variable is a GUI class right? If so than you are doing something you should never do in Qt: using GUI classes in other threads! GUI classes should only be used in main thread.
    If you want to trigger an UI change from other threads then just emit a signal there and connect a slot in your main thread where you then update the UI.



  • You are right.
    However, graphic->nextGraphicGen() doesn't change any GUI-Element. It calls only a method of GraphicCell, in which a Cell-method is called.
    GraphicalWindow (the class of graphic) inherits from QGraphicsView.
    But graphic->scene->update(graphic->scene->sceneRect()); should be in a slot in the GUI-Thread, that is right.
    I will fix this today evening.

    I hope you understand, what I mean.



  • This is the new code:

    #include "separatethread.h"
    #include <QApplication>
    #include <QMutex>
    
    SeparateThread::SeparateThread(GraphicalWindow* graphic, Settings* settings, Label *label, int &loops, QObject *parent) : QObject(parent), graphic(graphic), settings(settings), loops(loops)
    {
    	QObject::connect(this, SIGNAL(changedTimer(uint)), label, SLOT(changeTimer(uint)));
    	QObject::connect(this, SIGNAL(updateGraphic()), graphic, SLOT(sceneUpdate()));
    }
    
    void SeparateThread::work()
    {
    	if(loops == -1)
    	{
    		while(loops != -2)
    		{
    			mutex.lock();
    			graphic->nextGraphicGen();
    			emit changedTimer(graphic->getGeneration());
    			emit updateGraphic();
    			QThread::msleep(settings->getAutoplaySpeed());
    			mutex.unlock();
    		}
    	}
    	else if(loops == -3)
    	{
    		mutex.lock();
    		graphic->nextGraphicGen();
    		emit changedTimer(graphic->getGeneration());
    		emit updateGraphic();
    		loops = 3;
    		mutex.unlock();
    	}
    	else
    	{
    		for(int i = 0; i < loops && loops > 0; ++i)
    		{
    			mutex.lock();
    			graphic->nextGraphicGen();
    			emit changedTimer(graphic->getGeneration());
    			emit updateGraphic();
    			QThread::msleep(settings->getAutoplaySpeed());
    			mutex.unlock();
    		}
    	}
    }
    

    Does that look better?


Log in to reply
 

Looks like your connection to Qt Forum was lost, please wait while we try to reconnect.