The program has unexpectedly finished

dbzhang800

[quote author="Flurite" date="1335478572"]

When I click any of the buttons, the program becomes unresponsive.. this is unusual, as my slot functions were working completely fine before I decided to make a separate class for my timer[/quote]

If so, somewhere else must have a problem too.

In addition, lines 11-15 in you widget.cpp do nothing now.

@
Stopwatch* connectorClass = new Stopwatch;
connect(ui->Stopwatch_Start, SIGNAL(clicked()), connectorClass, SLOT(timer_Start()));
connect(ui->Stopwatch_Pause, SIGNAL(clicked()), connectorClass, SLOT(timer_Pause()));
connect(ui->Stopwatch_Reset, SIGNAL(clicked()), connectorClass, SLOT(timer_Reset()));
delete connectorClass;
@

Flurite

Oh yeah, that delete was pointless. Anyhow, where do you suspect the problem is?

MuldeR

You are doing strange things there: First you create a new Stopwatch object on the heap (with "new" command) and store the pointer in a local variable. That is okay, so far. Then you connect some signals to the newly created Stopwatch object by using your local pointer. Still okay. But then you delete the object? Huh? In the best case Qt will automatically remove the connection you just created when the object is destroyed. In the worst case it does not and you have a connection to a non-existing object. This is going to cause unexpected behavior and may even lead to application crash...

[EDIT] Sorry, 1+1=2 was faster ;-) [/EDIT]

MuldeR

[quote author="Flurite" date="1335479359"]Oh yeah, that delete was pointless. Anyhow, where do you suspect the problem is?[/quote]

You can't just remove the delete. When the function goes out of scope, you'll loose the pointer to your Stopwatch object, because connectorClass is a local variable. The object still exists, but you don't have any pointer to it. This will cause a memory leak, because you won't be able to delete it later...

I'd recommend to make connectorClass a member variable. And don't forget the delete in the deconstructor!

Flurite

Sorry, I think I was posting the wrong code. The program becomes unresponsive when I actually include DO NOT include the delete statement. When I do include the delete statement, the connections disconnect (I think..) and the program just doesn't do anything.

Late post: Alright, I will give your most recent post a shot

MuldeR

Please post full code...

dbzhang800

Line 4 in your stopwatch.cpp:
@
Ui::Clock_Application* ui;
@

is a wild pointer too.

I don't know why your application designed in such pattern. If you delete the class Stopwatch, and implement such function in Clock_Application, it will become very simple and natural.

MuldeR

I think Ui::Clock_Application* ui is initialized and deleted correctly.

@Clock_Application::Clock_Application(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::Clock_Application) // <---
{
ui->setupUi(this);

....@

dbzhang800

[quote author="MuldeR" date="1335480007"]I think Ui::Clock_Application* ui is initialized and deleted correctly.
[/quote]

Yes, but it has nothing with the ui used in your stopwatch.cpp.

Flurite

[quote author="1+1=2" date="1335479811"]Line 4 in your stopwatch.cpp:
@
Ui::Clock_Application* ui;
@

is a wild pointer too.

I don't know why your application designed in such pattern. If you delete the class Stopwatch, and implement such function in Clock_Application, it will become very simple and natural.[/quote]

I am going to have more slots in my application, and I just do not want to clutter up the Clock_Application class.. I think by dividing it into multiple files will make it simpler. Plus, does ui need to be defined if it's only used to access widgets?

MuldeR

The problem is, that the "ui" in your Stopwatch class, that you defined as global variable, is not the same "ui" as the one in your Clock_Application application class, which is a member variable of the Clock_Application class itself. Consequently the value of the "ui" in your Stopwatch class is just an un-initialized pointer. You never assigned a value to it! Any access to that "ui" will cause crash or undefined behavior...

Make the "ui" in your Stopwatch class a member variable of the Stopwatch class, rather than a global variable. Also you have to assign it to a value, so you have to pass through the "ui" from Clock_Application in the constructor of Stopwatch and then store it in the member variable...

@class Stopwatch : public QObject
{
Q_OBJECT

...

private:
QTimer* stopwatchTimer;
Ui::Clock_Application *ui; //here we'll store the actual ui-Pointer
};@

@Stopwatch::Stopwatch(Ui::Clock_Application *ui)
{
this->ui = ui; //Now store the ui-Pointer we got from the Clock_Application!
stopwatchTimer = 0;
}@

And in Clock_Application we do:

@ Clock_Application::Clock_Application(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::Clock_Application)
{
ui->setupUi(this);

    myStopwatch = new Stopwatch(ui); // <--- pass ui-Pointer over

    ....@

dbzhang800

[quote author="Flurite" date="1335480406"]

I am going to have more slots in my application, and I just do not want to clutter up the Clock_Application class.. I think by dividing it into multiple files will make it simpler.
[/quote]

That's OK.

But Stopwatch need to accept an private memeber of another Class now, so this is a bad design.

[quote author="Flurite" date="1335480406"]
Plus, does ui need to be defined if it's only used to access widgets?
[/quote]
No, Seems what you need is some basic knowledge of C++ ;-).

Flurite

[quote author="1+1=2" date="1335480901"]
[quote author="Flurite" date="1335480406"]

I am going to have more slots in my application, and I just do not want to clutter up the Clock_Application class.. I think by dividing it into multiple files will make it simpler.
[/quote]

That's OK.

But Stopwatch need to accept an private memeber of another Class now, so this is a bad design.

[quote author="Flurite" date="1335480406"]
Plus, does ui need to be defined if it's only used to access widgets?
[/quote]
No, Seems what you need is some basic knowledge of C++ ;-).[/quote]

I think you are trying to say that I am dereferencing a garbage pointer? I saw that problem from the start, but I didn't know how to fix it. When I tried to replicate how Qt implemented the ui pointer, I never saw a definition for it, so that's why I thought I didn't need one for mine. Anyhow, I think I see the definition in the widget.cpp file now.

MuldeR

[quote author="Flurite" date="1335481107"]I think you are trying to say that I am dereferencing a garbage pointer? I saw that problem from the start, but I didn't know how to fix it.[/quote]

You can do it, for example, like the code in my previous post:
http://qt-project.org/forums/viewthread/16697/#83748

Flurite

[quote author="MuldeR" date="1335481183"][quote author="Flurite" date="1335481107"]I think you are trying to say that I am dereferencing a garbage pointer? I saw that problem from the start, but I didn't know how to fix it.[/quote]

You can do it, for example, like the code in my previous post:
http://qt-project.org/forums/viewthread/16697/#83748[/quote]

Darn good idea. It's working now.. should have remembered using the this pointer to clarify some scope problems. Thanks!

MuldeR

Actually you could just have used other names:

@class Stopwatch : public QObject
{
Q_OBJECT

public:
Stopwatch(Ui::Clock_Application *bar);

...

private:
QTimer* stopwatchTimer;
Ui::Clock_Application *m_foo; //here we'll store the actual ui-Pointer
};

.......

Stopwatch::Stopwatch(Ui::Clock_Application *bar)
{
m_foo = bar; //Now store the ui-Pointer we got from the Clock_Application!
stopwatchTimer = 0;
}

.......

void Stopwatch::timer_Reset()
{
timer_Pause();
m_foo->Stopwatch_Output->setText("00:00:00");
}@

@ Clock_Application::Clock_Application(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::Clock_Application)
{
ui->setupUi(this);

    myStopwatch = new Stopwatch(ui); // <--- pass ui-Pointer over

    ....@

The name of your pointer variables doesn't matter. It matters that you store the right pointer!

Flurite

Yes, yes, I understand that. In fact, I thought of that right after I replied to your post. I don't know why, but I just have this need for a unique consistency in all my codes.

dbzhang800

[quote author="MuldeR" date="1335483291"]
The name of your pointer variables doesn't matter. It matters that you store the right pointer![/quote]

In fact, we can use the same name even without this

@
Stopwatch::Stopwatch(Ui::Clock_Application *ui)
:ui(ui)
{
//this->ui = ui;
@

But as I said in previous post, current design is rather bad. it will made the application more complex and difficult to maintain.

[quote author="Flurite" date="1335480406"]
I am going to have more slots in my application, and I just do not want to clutter up the Clock_Application class.. I think by dividing it into multiple files will make it simpler.
[/quote]

MuldeR

I agree.

If possible, the Stopwatch class shouldn't need to know about the whole Ui::Clock_Application class. I'd prefer to only pass a pointer to a QLabel object (or whatever type the Stopwatch_Output is) around, which makes the Stopwatch class more general, more reusable and less likely to be effected by changes in "far away" code...

(But I wouldn't say that wrapping the "Stopwatch" functionality into its own class is bad per se. An alternative worth considering would be making the Stopwatch class inherit from QLabel or another Widget class though)

@class Stopwatch : public QObject
{
Q_OBJECT

public:
Stopwatch(QLabel *output);

...

private:
QTimer* stopwatchTimer;
QLabel *m_output; //here we'll store the pointer to the Widget
};

...

Stopwatch::Stopwatch(QLabel *output)
{
m_output = output; //Now store the pointer to the Widget
stopwatchTimer = 0;
}

...

void Stopwatch::timer_Reset()
{
timer_Pause();
output->setText("00:00:00");
}@

@ Clock_Application::Clock_Application(QWidget *parent) :
QMainWindow(parent),
ui(new Ui::Clock_Application)
{
ui->setupUi(this);

        myStopwatch = new Stopwatch(ui->Stopwatch_Output); // <--- pass Widget
 
        ....@

juliocnl_92

I remember that the timer must be stopped before closing the window, i had that problem before, and i resolved it by calling the 'stop' method when the program is closing.