[SOLVED] Get all Objects with same class ?

SGaist

As for every user of the Qt library

STL is a different beast

@CONFIG += c++11@

sets everything needed to use C++11

deleted28

I'm sorry but i need more help.
I do not know how to work on the QSet instances.

knhead.h:
@#include <QObject>
#include <QSet>

class KNhead
{
public:
KNhead();
~KNhead();

QSet<KNhead*> instances;

};@

knhead.cpp:
@#include "knhead.h"

QSet<KNhead*> instances;

KNhead::KNhead()
{
instances.insert(this);
}

KNhead::~KNhead()
{
instances.remove(this);
}@

main.cpp:

@#include <QCoreApplication>
#include <QDebug>
#include <QSet>
#include "knhead.h"

int main(int argc, char *argv[])
{
QCoreApplication a(argc, argv);
qDebug() << "Hello\n";

KNhead *cell_1 = new KNhead;
KNhead *cell_2 = new KNhead;
KNhead *cell_3 = new KNhead;

// qDebug() << ??? instances.count() ??? ;

qDebug() << cell_1->instances.count();
qDebug() << cell_1->instances.size();

qDebug() << cell_2->instances.count();
qDebug() << cell_2->instances.size();

return a.exec(&#41;;

}@

output:
@Hello

1
1
1
1
@

Is there a "static" missing ?
or do i need ust some more coffe to wake up this morning

JKSH

[quote author="wally123" date="1422779095"]Is there a "static" missing ?[/quote]Yes, your set needs to be a static variable. You want all the different KNhead instances to write to the same set, after all.

deleted28

just put a "static" in knhead.h line 10 and knhead.cpp line 3
@
static QSet<KNhead*> instances;
@

results in:

error: undefined reference to `KNhead::instances'

JKSH

You need to define static member variables in a .cpp file somewhere. Add this line to knhead.cpp (outside of any functions):

@
QSet<KNhead*> KNhead::instances;
@

Do some extra reading to understand "how to use static member variabless":http://www.learncpp.com/cpp-tutorial/811-static-member-variables/

deleted28

Ok, now no errors and it s output looks good:
@Hello

3
3
3
3@

Do i need always an arbitrary instance of class KNhead to access instances
or is there another way ?

e.g. qDebug() << cell_1->instances.count();
or cell_2 or cell_3

gyll

As i told you in my first reply (you forgot :P):
"Once you have the above, you have access to ‘instances’ anywhere in your program (provided you #include “custom.h” in the cpp files where you use ‘instances’) via name qualification: 'Custom::instances'

So, in any cpp file where you want to access 'instances' just make sure to #include "knhead.h" and then just use 'KNhead::instances':
@
#include "knhead.h"
...
qDebug() << KNhead::instances.count();
...
@

JKSH

[quote author="gyll" date="1422783535"]
@
#include "knhead.h"
...
qDebug() << KNhead::instances.count();
...
@
[/quote]It's a good idea to avoid globally-accessible variables where feasible. Currently, someone can add/remove items from the set, without constructing/destroying a KNhead!

For more robust code, make the set private and implement a static member function that allows you to read the variable (but not modify it):

@
class KNhead
{
public:
static QSet<KNhead*> KNhead::instances() {
return m_instances;
}

private:
QSet<KNhead*> m_instances; // Give it a different name from the function
}
@

gyll

w/r JKSH: using getters in the situations where you want to very clearly isolate the places where a data member changes is indeed considered the "clean code good practice" for ordinary data members, but if you have:

you want to provide a static getter method
you don't want to make the getter method return the actual variable because of efficiency reasons (in this particular example the getter would return a copy of the entire set each time it's called, i.e. alloc/dealloc)

then you'd need to make the getter return a reference to the variable, and you'd need to make the getter 'const'. but you can't have const qualifiers for static methods:

@
static QSet<KNhead*>& instances() const; // wrong: can't specify 'const' to static methods
@

Alternatively, one could provide method wrappers for all the methods that are needed in your code in conjunction with the data member (it's the cleanest way, but it takes a bit of typing and code checking), e.g. you could have a static method instanceCount(), instanceList(), etc, and use one method or another as needed in the code (sure enough, instanceList() would still return a copy of the set, but only when the full set is actually needed e.g. for iterating through it)

PS
fwiw, i personally don't bother much with write protection (not even for ordinary data members) except when writing interfaces for objects that are to be used by "the general public"; inside the nuts and bolts my own code i just let it be

deleted28

@class KNhead
{
private:
QSet<KNhead*> m_instances;

public:
KNhead();
~KNhead();

// static QSet<KNhead*> instances;

static QSet<KNhead*> instances() {
    return m_instances;
}

int testProp;

};@

/knhead.h:4: error: invalid use of member 'KNhead::m_instances' in static member function
QSet<KNhead*> m_instances;

gyll

you should have spotted the mistake yourself by now :P

there's a little error there, it should be:

private:
static QSet<KNhead*> m_instances;

i.e. the member still has to be static, just as it was until now, you only added a new static getter method but everything else remains the same (static methods can only access static data members of an object).
still, i have to say it again, using getters (not to mention static getters) is in my opinion an overkill when it's not a "general public" object. If i were you i'd just use the variable 'KNhead::instances' directly where i needed it.

deleted28

If i understnad you correctly, you recomment to use this QSet<T*>
( set of gathered objects created by gather method (?) )
as public and not to use JKSH's suggestion to make it private
and not avoid globally-accessible variables for now, because
to much efforts to implement it (overkill).
Is this correct ?

SGaist

No, you are just missing the static qualifier in from of m_instances, JKSH's technique is fine

deleted28

I set the static already and it works now.
My above question is meant to this :

bq. still, i have to say it again, using getters (not to mention static getters) is in my opinion an overkill when it’s not a “general public” object. If i were you i’d just use the variable ‘KNhead::instances’ directly where i needed it.

JKSH

[quote author="wally123" date="1422790846"]If i understnad you correctly, you recomment to use this QSet<T*>
( set of gathered objects created by gather method (?) )
as public and not to use JKSH's suggestion to make it private
and not avoid globally-accessible variables for now, because
to much efforts to implement it (overkill).
Is this correct ?
[/quote]Yes, that is what gyll recommended.

Both approaches work.

The global approach is the "fast" way, and saves you a few lines of code at the start
The the private approach is the "safe" way, and reduces the risk of you making mistakes in the future.

As the program designer, you need to compare the costs and benefits of the different approaches and decide which one you want.

I leave you with this piece of wisdom from xkcd:
!http://imgs.xkcd.com/comics/good_code.png(Good Code)!
(source: http://xkcd.com/844/ )

deleted28

nice !
now the Iterator ... :)

deleted28

this works to find by variable name:
@
QSet<KNhead*>::iterator it = qFind(KNhead::instances.begin(), KNhead::instances.end(), cell_2);@

I introduced a public property to KNhead class as
@int testProp;@

@KNhead *cell_2 = new KNhead;
cell_2->testPop = 222;
@

Can i find/search the object in the entire QSet by this property ?
Means, find an object in KNhead::instances with testProp == 222 ?

Of course i can go through each object an check each on the contents of the testProp,
i want to ask if there is a direct possibility by STL or Qt .

JKSH

If you want to search by one particular property, use a QMap instead of a QSet, and use the property as the map key.

@
// Declaring the map
QMap<int, KNhead*> instances;
@

@
// Inserting into the map
KNhead::KNhead(int prop)
{
this->testProp = prop;
instances[prop] = this;

// ASSUMPTION: The value of this property never changes

}
@

@
// Retrieving from the map
KNhead *cell = instances[222];
@

If you want to search by multiple properties, use an in-memory SQLite database.

gyll

just a quick correction to my last reply, dunno what was in my mind when i wrote it: i said that if declaring a static getter it should be declared as 'const', and this is not possible for static methods. however, i was wrong: in order to prevent using the getter method for changing the data value, it is the return value that should be declared as 'const', not the method itself.

So it is entirely possible to declare a clean getter method which returns a const reference:

@
// knhead.h
#include <unordered_set>

class KNhead
{
static std::unordered_set<KNhead*> m_instances;
public:
KNhead();
~KNhead();
static const std::unordered_set<KNhead*>& instances();
};

// knhead.cpp
#include "knhead.h"
#include <assert.h>

std::unordered_set<KNhead*> KNhead::m_instances;

KNhead::KNhead() {
m_instances.insert(this);
}

KNhead::~KNhead() {
assert(m_instances.count(this));
m_instances.erase(this);
}

const std::unordered_set<KNhead*>& KNhead::instances() {
return m_instances;
}
@

With the KNhead::instances() as defined above you're safe (the compiler won't let you do something like e.g. KNhead::instances().insert(x) , it will complain about you trying to change a const reference)

deleted28

great ! thanks a lot

and it works !