Problem bei einem Growable Array

adonis

Hallo ich habe mir eine Array Klasse geschrieben die dynamisch wächst, soweit
ganz gut, aber ich hab das Objekt nicht wieder rausbekommen, ich
habe mir die Speicherbereiche angeguckt und gesehen dass die nicht passen
das 0-te Element liegt bei mir auf Index -1, ich finde den Fehler nicht.

Hier meine Array Klasse:

#include "TArray.h"
#include <malloc.h>

TArray::TArray(int Length)
{
	m_iCount = 0;
	m_iLenght = 0;
	m_iResizeVector = 0;	
	m_bResizable = false;
	m_lpVoid = (void**)malloc(Length*sizeof(void*)+1);
	m_iLenght = Length;
}

TArray::~TArray(void)
{
	for(int i = 0; i < m_iLenght; i++)
	{
		free(m_lpVoid[m_iLenght]);
	}

}

void TArray::Add(void* Element)
{	
	if(m_iCount < m_iLenght)
	{
		m_lpVoid[m_iCount] = Element;
		m_iCount++;
	}
	if(m_bResizable && m_iCount == m_iLenght && m_iResizeVector > 0)
	{
		m_iLenght += m_iResizeVector;
		realloc(m_lpVoid, m_iLenght*sizeof(void*)+1);
		this->Add(Element);
	}
}

void TArray::Remove(void* Element)
{
	if(m_iCount > 0)
	{
		for(int index = 0; index < m_iCount; index ++)
		{			
			if(m_lpVoid[index] == Element)
			{
				m_lpVoid[index] = NULL;
				ReArrange(index);
				m_iCount--;
				Remove(Element);
			}
		}
		return;
	}
	return;
}

void TArray::RemoveAt(int index)
{
	if(index < m_iCount && m_lpVoid[index] != NULL)
	{
		m_lpVoid[index] = NULL;
		m_iCount--;
	}
}

void* TArray::GetItemAtIndex(int index)
{
	if(index < m_iCount)
		return m_lpVoid[index];
	else
		return NULL;
}

void TArray::ReArrange(int pos)
{
	for(int arrange = pos; arrange < m_iCount-1; arrange++)
	{
		m_lpVoid[arrange] = m_lpVoid[arrange+1];
	}
}

void TArray::SetResizeable(bool resizeable)
{
	m_bResizable = resizeable;

}
bool TArray::GetResizeable()
{
	return m_bResizable;
}
void TArray::SetResizeVector(int resizevector)
{
	m_iResizeVector = resizevector;
}
int TArray::GetResizeVector()
{
	return m_iResizeVector;
}

int TArray::Count()
{
	if(m_iCount)
		return m_iCount;
}

int TArray::Lenght()
{
	return m_iLenght;
}

hier der Fall:

//////Test Area//////
	//TComponent* root = new TComponent(NULL);
	//TComponent* Child1 = new TComponent(root);
	//TComponent* Child2 = new TComponent(root);
	//TComponent* Child3 = new TComponent(root);

	//int x = root->getTreeSize();
	TArray* arr = new TArray(10);
	TObject * o = new TObject();
	o->SetTClassName(EnumClassName::ClassEnum::T3DDevice);
	arr->Add(o);

	TObject* oo = ((TObject *)&arr[-1]); <-- Warum hier -1????
	const char* c = (oo)->ClassNameToString(oo->GetTClassName());

	/////////////////////

Danke

Swordfish

...

Du hast in deinem Klassencode die Überladung des operator[] nirgends aufgeführt. Daher wird dir beidem speziellen problem niemand helfen können.

adonis

ok, hier die TArray.h

#pragma once
#include "BaseUsings.h"
#include "TBaseClass.h"
class TArray
{
private:
	void** m_lpVoid;
	int m_iCount;
	int m_iLenght;
	int m_iResizeVector;
	bool m_bResizable;
	void ReArrange(int);
public:
	TArray(int);
	~TArray(void);
	void Add(void*);
	void Remove(void*);
	void RemoveAt(int);
	void* GetItemAtIndex(int);
	void SetResizeable(bool);
	bool GetResizeable();
	void SetResizeVector(int);
	int GetResizeVector();
	int Count();
	int Lenght();
};

otze

*keuch und sterb*

Swordfish

...

camper

Namenloser324 schrieb:

Du hast in deinem Klassencode die Überladung des operator[] nirgends aufgeführt. Daher wird dir beidem speziellen problem niemand helfen können.

Überladung ist hier nicht im Spiel.

TArray* arr = new TArray(10);
    TObject * o = new TObject();
// ...

//    TObject* oo = ((TObject *)&arr[-1]); <-- Warum hier -1????
// äquivalent zu
    TObject* oo = reinterpret_cast<TObject*>(&*(arr-1));  // wahrscheinlich Verwaltungsdaten im free store

in jedem Fall Unfug

adonis

TArray soll nicht nur Elemente eines Types halten.
Ich habe eine Basisklasse TComponent die TArray enthält.
ich möchte ettliche Componenten schreiben und einen Componenttree aufbauen.

Ich habe gesehen ich muss noch defensiver arbeiten.

Ich möchte derzeit nicht vector oder cgrowable von Microsoft nutzen,
ich will es lernen und verstehen. Leider sind meine C++ Kenntnisse noch
nicht so gut.

Ich werd die Klasse noch einmal Schritt für Schritt durchgehen und Defensiver arbeiten, einige Schönheitsfehler sind mir noch aufgefallen.

Danke Trozdem

Wie willst du ein TArray benutzen, wenn du nicht weißt, welchen Typ ein Element hat? Casten und hoffen? Das ist doch Murks.

Halt einen std::vector<std::unique_ptr<TComponent>> oder so und sorg dafür, dass TComponent alle Operationen beherrscht, die du für den component tree brauchst. Und verwirf deinen bisherigen Ansatz -- egal, wie defensiv du den umsetzt, du polierst da Katzengold.

adonis

Alle meine Klassen haben eine Basisklasse, ich frag einfach um welchen Type es
sich handelt.

Swordfish

...

adonis

Also TObject is meine Mutter aller Klassen, und hält den Classname.
Dann kann ich schon Fragen, um wen es sich handelt und dann weiß ich auch was das
Object kann, oder nicht?

Willst du jedes Mal, wenn du eine neue Komponente baust, allen Code anfassen müssen, der TArray verwendet?

Ich lass mal nen Link da: https://de.wikipedia.org/wiki/Liskovsches_Substitutionsprinzip

Ich rate dir dringendst, das in deinem Design zu berücksichtigen, und zwar ganz unabhängig von der Programmiersprache. Bau dir eine Basisklasse, die die Operationen hat, die du brauchst, und dann merk dir, dass du Objekte dieses Typs verwaltest. Typinformationen zu vergessen und später wieder auszuklamüsern ist kein sinnvoller Ansatz.

adonis

seldon schrieb:

Willst du jedes Mal, wenn du eine neue Komponente baust, allen Code anfassen müssen, der TArray verwendet?

Verstehe ich nicht ganz, nur 2 Klassen haben TArray und zwar die Basis Komponente
von der alle Komponenten erben und eine Kindklasse von der alle Visuellen Komponenten erben werden, um einen SceneGraph aufzubauen.

Ich werd mir den link mal anschauen danke

krümelkacker

adonis schrieb:

Hallo ich habe mir eine Array Klasse geschrieben [...]

hier die TArray.h

#pragma once
#include "BaseUsings.h"
#include "TBaseClass.h"
class TArray
{
private:
	void** m_lpVoid;
	int m_iCount;
	int m_iLenght;
	int m_iResizeVector;
	bool m_bResizable;
	void ReArrange(int);
public:
	TArray(int);
	~TArray(void);
	void Add(void*);
	void Remove(void*);
	void RemoveAt(int);
	void* GetItemAtIndex(int);
	void SetResizeable(bool);
	bool GetResizeable();
	void SetResizeVector(int);
	int GetResizeVector();
	int Count();
	int Lenght();
};

TArray::TArray(int) sollte explizit sein.
Viel zu viel void ! Du schmeißt wertvolle Typinformation weg.
Hier und da fehlt const .
Verletzung der Dreierregel! (Bitte selbst nachgucken, was das ist)

adonis schrieb:

Hier meine Array Klasse:

#include "TArray.h"
#include <malloc.h>

TArray::TArray(int Length)
{
	m_iCount = 0;
	m_iLenght = 0;
	m_iResizeVector = 0;	
	m_bResizable = false;
	m_lpVoid = (void**)malloc(Length*sizeof(void*)+1);
	m_iLenght = Length;
}

TArray::~TArray(void)
{
	for(int i = 0; i < m_iLenght; i++)
	{
		free(m_lpVoid[m_iLenght]);
	}
}

...

Speicherleck: Es fehlt ein free(m_lpVoid) im Destruktor
Du versuchst hier die Pointees zu mit free löschen, ohne zu wissen, was eigentlich hinter den void*-Zeigern steht. Ganz schlechte Idee! So werden beispielsweise garantiert keine Destruktoren aufgerufen.

adonis schrieb:

hier der Fall:

//////Test Area//////
	//TComponent* root = new TComponent(NULL);
	//TComponent* Child1 = new TComponent(root);
	//TComponent* Child2 = new TComponent(root);
	//TComponent* Child3 = new TComponent(root);

	//int x = root->getTreeSize();
	TArray* arr = new TArray(10);
	TObject * o = new TObject();
	o->SetTClassName(EnumClassName::ClassEnum::T3DDevice);
	arr->Add(o);

	TObject* oo = ((TObject *)&arr[-1]); <-- Warum hier -1????
	const char* c = (oo)->ClassNameToString(oo->GetTClassName());

	/////////////////////

Zuviel new . Wozu legst du Dein Array im Heap an?
Speicherlecks: Nirgens wird irgend etwas freigegeben.
Wenn du Dein Array statt im Heap einfach im automatischen Speicher anlegst, wird es automatisch freigegeben. Dann läuft der Destruktor, der aber die per new erzeugten Objekte mit free freigegben will. Das ist falsch. Zu einem new gehört ein delete . Zu einem new[] gehört ein delete[] . Allerdings muss der Zeigertyp beim delete schon der richtige sein: Entweder der einer Basisklasse mit virtuellem Destruktor oder der richtige dynamische Typ.

Ich will Dich da nicht demotivieren, aber das, was du da gebaut hast, ist längst nicht so toll, wie Du es Dir wahrscheinlich vorgestellt hast. Es sieht für mich so aus, als hättest Du großes Pech mit der Auswahl Deines Lehrmaterials gehabt. Ich würde Dir empfehlen, den Kram zu vergessen und nochmal von vorn anzufangen, ohne malloc, ohne free, ohne new, ohne delete. Ich würde Dir empfehlen, auch wenn Du noch nicht weißt, wie die Sachen aus der Standardbibliothek intern funktionieren, doch erst mal diese Sachen zu verwenden, um ein "richtiges C++ Gefühl" zu bekommen. Später kann man sich dann angucken, wie man so etwas selbst schreibt.

Die C++ Community hat Dir gegenüber einen großen Lern-Vorsprung. Das ist jetzt nichts schlimmes. Du musst nur nicht jeden Fehler wiederholen, die andere schon vor Dir gemacht haben. Vor der aktuellen Standardbibliothek gab es u.a. Container-Bibliotheken, die so ähnlich gestrickt waren wie Deine. Und die sind zu Recht ausgestorben.

MichelRT

Kann das sein, dass Du früher mit Delphi gearbeitet hast? Namensgebung und Konzept erinnern an die VCL.

In C++ kann man im Gegegsatz zu Delphi Objekte nicht nur dynamisch auf dem Heap anlegen sondern auch "automatisch" auf dem Stack. Außerdem gibt es auch so nette Sachen wie Smartpointer. Wenn Du es richtig machst und die Tips aus den Beiträgen hier befolgst musst Du dich bei C++ um die Speicherverwaltung Deiner Objekte nicht selber kümmern.