QHash insert() ?

[huckleberry]

Hallo Foristen,

ich habe hier eine einfache Klasse mit einigen Attributen und Get-Methoden. Wirklich nichts besonderes:

Code: Alles auswählen

class plItem
{
public:
	plItem();
	plItem(q_int32 tId, q_int32 tLon, q_int32 tLat);
	plItem(q_int32 tId, QString tName, q_int32 tCat, q_int32 tLon, q_int32 tLat);
	~plItem(void);

	q_int32	getID();
	QString	getplName();
	q_int32	getplCat();
	q_int32	getplLon();
	q_int32	getplLat();

private:
	q_int32	plID;
	QString	plName;
	q_int32	plCat;
	q_int32	plLon;
	q_int32	plLat;
};

Dann habe ich eine Container-Klasse gebastelt, welche eine QHash hat, welche wiederrum eine ID zu plItem's mappt, also QHash<q_int32, plItem>:

Code: Alles auswählen

#pragma once

#include <QtGui>
#include <QString>
#include <QHash>
#include "plItem.h"

class PL_Controller
{
public:
	PL_Controller(void);
	~PL_Controller(void);

	void addNewPL(q_int32, QString, q_int32, q_int32, q_int32);

	

private:
	
		QHash<q_int32, plItem> dynamicPOIs;
		
};

Und die cpp:

Code: Alles auswählen

#include "PL_Controller.h"

PL_Controller::PL_Controller(void)
{
	dynamicPOIs = new QHash();
}

void PL_Controller::addNewPL(pi_int32 tID, SDString tName, pi_int32 tCat, pi_int32 tLon, pi_int32 tLat)
{
	//
	plItem tPI = new plItem(tID, tName, tCat, tLon, tLat);
	this->dynamicPOIs.insert(tID, tPI);
}

POI_Controller::~POI_Controller(void)
{
}

Ich bekomme folgende Fehlermeldung:

1>------ Build started: Project: AT_mod_Pl, Configuration: Debug Win32 ------
1>Compiling...
1>PL_Controller.cpp
1>.\PL_Controller.cpp(5) : error C2955: 'QHash' : use of class template requires template argument list
1> c:\qt\qt-win-opensource-src-4.5.0\include\qtcore\../../src/corelib/tools/qhash.h(253) : see declaration of 'QHash'
1>.\PL_Controller.cpp(5) : error C2512: 'QHash' : no appropriate default constructor available
1>.\PL_Controller.cpp(11) : error C2440: 'initializing' : cannot convert from 'plItem *' to 'plItem'
1> No constructor could take the source type, or constructor overload resolution was ambiguous
1>CPLDockWidget.cpp
1>M:\Eigene Dateien\project\srcz\AT_plugins\AT_mod_Pl\CPLDockWidget.cpp(97) : error C2248: 'CModPl::importPls' : cannot access private member declared in class 'CModPl'
1> m:\eigene dateien\project\srcz\AT_plugins\AT_mod_Pl\CModPl.h(127) : see declaration of 'CModPl::importPls'
1> m:\eigene dateien\project\srcz\AT_plugins\AT_mod_Pl\CModPl.h(45) : see declaration of 'CModPl'
1>Generating Code...
1>Build log was saved at "C:\Eigene Dateien\project\build-vs9-wPl\AT_mod_Pl\AT_mod_Pl.dir\Debug\BuildLog.htm"
1>AT_mod_Pl - 4 error(s), 0 warning(s)
========== Build: 0 succeeded, 1 failed, 2 up-to-date, 0 skipped ==========

Ich danke für alle Hinweise!

Achja: Visual Studio 2008 mit Qt.

Mfg Huck

[franzf]

Code: Alles auswählen

QHash<q_int32, plItem> dynamicPOIs; 
...
dynamicPOIs = new QHash(); 
...
plItem tPI = new plItem(tID, tName, tCat, tLon, tLat); 
this->dynamicPOIs.insert(tID, tPI); 

Du programmierst hier Java und nicht C++. new brauchst du in C++ nur zum Initialisieren von Zeigern. dynamicPOIs ist kein Zeiger. Selbiges gilt fürs Einfügen deines plItem in den Hash.
"plItem tPI = new ..." geht aus obigem Grund ebenfalls nicht. Korrekt wäre

Code: Alles auswählen

plItem tPI(tID, tName, tCat, tLon, tLat);

dynamicPOIs musst du erst gar nicht selber konstruieren, das Objekt wird wenn du es nicht selber aufrufst sowieso per Default-Konstruktor initialisiert, dehalb reicht ein leerer Konstruktor.

[huckleberry]

Danke!

[huckleberry]

Hm,

Code: Alles auswählen

ptrMyValue * MyContainer::getValue(q_int32 tID)
{
	if ( this->dynamicPOIs.contains(tID) ) 
		return this->dynamicPOIs.value(tID);
	else return 0;
}

Ich möchte ja einen Zeiger zurück geben..

error C2440: 'return' : cannot convert from 'const returnValue' to 'returnValue *'

Normalerweise schiebe ich ein & davor, wenn ich dessen Adresse haben will, aber irgendwas mache ich falsch?

Hat jemand Hinweise? Danke.

Mfg Huck

[Christian81]

Was macht die Klasse MyContainer? Wie ist dynamicPOIs definiert?
Des weiteren ist das contains überflüssig.

[huckleberry]

Wie ist dynamicPOIs definiert?

Code: Alles auswählen

QHash<q_int32, MyValue> dynamicPOIs;

Ich habe ein <int, MyValue> Objekt Paar als QHash. MyValue ist wiederum eine komplexe Klasse mit wenigen int, string Attributren.
Obacht: das MyValue hat kein Sternchen*.
Mit der Methode MyValue * MyContainer::getValue() soll dann ein Zeiger auf solch ein Wert (welches sich ja im QHash befindet) zurückgegeben werden.

Was macht die Klasse MyContainer?

MyContainer ist die Klasse, welche mein QHash verwaltet. Ich habe diese Klasse gebaut, um es später bequem erweitern zu können.

[Christian81]

Was brauchst Du noch was QHash nicht kann? Naja egal.
Du hast ein Objekt im Hash, also wäre ein '&' schon korrekt. Da value() aber eine Kopie des Objektes zurückliefert würde es crashen. Es gibt aber noch den [] operator

[huckleberry]

Mit operator []:

Code: Alles auswählen

return dynamicPOIs[tID];

error C2440: 'return' : cannot convert from 'returnValue' to 'returnValue *'

Obwohl die Doku

T & QHash::operator[]...

schriebt?!

[Christian81]

Und wo ist jetzt das Problem? Im Hash ist ein Objekt, also liefert der operator[], wie in der Doku steht, eine Referenz auf das Objekt zurück. Und Du möchtest einen Pointer zurückliefern.
Wie gesagt - QHash allein nochmal zu kapseln ist m.E. sinnlos bzw. nur in sehr speziellen Fällen nötig.

[RHBaum]

Wenn die definition deines Containers so aussieht:

QHash<q_int32, plItem> dynamicPOIs;

plItem kein Zeiger, sondern ne Instanz

um daraus nen Zeiger zu machen, klar brauchst den Addressoperator ...

die getValue() methode sollte dann in der art aussehen

Code: Alles auswählen

const plItem * MyContainer::getValue(q_int32 val) const
{
    const plItem * preturn = NULL;
    QHash<q_int32, plItem>::const_iterator itf = dynamicPOIs.find(val);
    if(itf != dynamicPOIs.end())
    {
        preturn = &itf.value();
    }
    return preturn;
}

Alles andere waere bissi gefaehrlich ....
nen nicht konstanten Pointer auf den Inhalt einer Klasse rausgeben, resultiert fast immer in lifecycleProbleme ....

Und ich wuerd mir echt ueberlegen, ob der Zeiger nach aussen sein muss ...
und ob deine plItem Klasse irgendwie ne ungültigkeitskennung hat ...
dann wuerde auch sowas gehen :

const plItem & MyContainer::getValue(q_int32 val) const

Ciao ...

[huckleberry]

Danke, so habe ich es gelöst.

Kurz zum Verständnis:
1.a) Das erste const signalisiert, Rückgabe ist Zeiger auf konstantes Objekt. Also das Objekt auf das das mein Rückgabezeiger zeigt ist konstant. Jedoch könnte später im Code, mein Zeiger auf ein anderes Objekt (desselben Typs) zeigen?
1.b) Stünde dort

Code: Alles auswählen

plItem * const MyContainer::getValue(q_int32 val) const {}

so wäre mein Rückgabewert ein konstanter Zeiger auf solch ein Objekt. D.h. das Objekt selbst kann sich im Laufe verändern, jedoch zeigt der Zeiger dann nur auf dieses Objekt.
1.b.1) Wie lange ist die Lebensdauer?
2) Das letzte const signalisiert nur, dass das Aufrufende Objekt nicht verändert wird und konstante (freilich auch nichtkonstante) Objekte diese Methode aufrufen dürfen.

Danke!

[RHBaum]

1a)

plItem * const MyContainer::getValue(q_int32 val) const {}

ist Bloedsinn, weil:
Dein Rueckgabewert ist immer eine Kopie. egal was du damit machen willst, Du kannst "von aussen" den rueckgabewert nicht ueberschreiben.

Ansonsten muesste sowas gehen:

int myFunc()
{
return 5;
}

/// irgendwo im Quelltext:
myFunc() = 7 ;

Jeder ernstzunehmende Compiler sollte da seinen Mund aufmachen.

Also den "eigentlichen" Rueckgabewert const zu machen ist sinnlos. Der rueckgabewert wird sowieso sofort was anderem zugewiesen, oder einfach ignoriert. Was anderes kannst ja ned machen.

std::cout << myFunc(); /// Ist auch eine impliziete Zuweisung (Parameter!).

was an der Stelle const machen kannst, sind nur die Ziele von referenzierten Daten. Also Zeiger und Referenzen !

1.b)
Die Lebensdauer ist so lange, wie deine "Referenz" in dem Ursprung Lebt. abgesichert ist na nix

Beispiel:

const plItem & myRef = myCont.getValue(y);
myCont.clear();
/// myRef kann hier ganz boese ins Abseits zeigen. Dafuer bist du, bzw die Prpgrammlogic Verantwortlich

Deshalb sollte man immer wenn die Klassen einigermassen trivial zu kopieren sind, Kopien benutzen.

plItem myValue = myCont.getValue(y); /// Macht intern ne Kopie ueber CCTor !
/// es intressiert dich ned mehr ob deine Ref noch gültig ist, weil du hasst ja deine eigene Kopie ....

2)
formulier es lieber so:

und konstante (freilich auch nichtkonstante) Objekte diese Methode aufrufen dürfen.

die Methode auch an konstanten Objekten aufgerufen werden darf.
laesst du das const weg :

const MyContainer myCont = ..../// irgendwas

const pItem * myItem = myCont.getValue(y); /// hier sollte der compiler dann maulen.

BTW:
pItem ist nen auesserst bescheuerter Name fuer ne Klasse !!!
Ich schreib immer p davor, wenn ich mit Zeiger auf Instanzen arbeite.
ich wuerd instinktiv nen lokalen Zeiger auf irgend ein Item auch pItem nennen wollen ... Das tut richtig weh, da staendig aufzupassen das ich ned mit dem KlassenNamen kollidiere

Ciao ...

[huckleberry]

pItem ist nen auesserst bescheuerter Name fuer ne Klasse !!!

Ja habs korrigert, ich weiss Klassennamen Gross anfangen. Kleines p an Anfang heisst auch bei mir Pointer

Ich danke dir.

[huckleberry]

Nochmal zum Problem ganz oben. Es klappt doch nicht ganz.

Die Zeile mit

Code: Alles auswählen

const ItemData * const tP = dDlg.getData();

funktioniert. Das Objekt auf das tP zeigt, hat auch korrekte Werte im Debugger.
In der nächsten Zeile mit addNew() gibts es Probleme, anscheinend klappt das Adden in die QHash nicht so sauber?

Code: Alles auswählen

void DataContainer::addNew(q_int32 tID, QString tName, q_int32 tCat, q_int32 tCon)
{
	//
	PoiItem tPI (tID, tName, tCat, tCon);
	this->myQHash.insert(tID, tPI);
}

Danke im voraus.

Achja: Nach dem adden "delete tP;"

Code: Alles auswählen

int MyDockWidget::addDataToQHash()
{
	//
	MyDialog dDlg(tr("Enter dynamic data"), this);
	if (dDlg.exec()==QDialog::Accepted){
		const DataItem * const tP = dDlg.getData();
		pDataContainer->addNew(tP->getID(), tP->getName(), tP->getCat(), tP->getCon());
		delete tP;
		return 1;
	}else return -1;

}

um den Speicher freizugeben richtig?

Mfg Huck

[RHBaum]

Ob Du tP wieder loeschen musst, haengt davon ab, was und wie dDlg.getData(); rausgibt.
Da sowas nie jemand weis, gibts da immer Problemchen.

Deshalb: Eine der Grundsätzlichsten Regel zur sicherheit des Speichers unter C++ lautet:
new und delete immer im Selben Context !

selber Context kann sein:
- innerhalb eines Codeblocks (funktion / Methode)
- Construktor /Destruktor
- create und release funktions Paare

Also bloed ist:

Code: Alles auswählen

const DataItem * const tP = dDlg.getData(); 
delete tP;  

Wenn dann

Code: Alles auswählen

const DataItem * tP ) dDlg.createData();
dDlg.releaseData(tP); 

wenn schon mit rohen zeigern.

Besser: Smartpointer in c++ nutzen !

ciao