Eure Meinug zum Code

[jeans]

Hallo, ich habe schon etwas über C/C++ gelernt. Nun, um das ganze zu festigen wollte ich meine eigene String-Klasse programmieren.

Würdet ihr auch das ansehen und eure Meingung sagen? Vor allem den Copyconstruktor funktioniert glaube ich nicht ganz richtig. Weil bei ein paar Tests das Programm immer wieder abgestürzt ist. Mit echt seltsamen Fehlermeldungen.

#ifndef _parsx_h_
#define _parsx_h_

int convert(char c);

namespace prsx
{
	int len(const char* st);
	int convert(char *c);

	class string{
	private:

		//
		// Member
		//
		///////////////////////////////////////////////////////////
		char* m_string;
		int   m_counter;

		//
		// private Methoden
		//
		///////////////////////////////////////////////////////////
		void cpychar(const char*);

	public:

		//
		// Konstruktor/en
		//
		///////////////////////////////////////////////////////////
		string ();
		string (const char*);
		string (const string&);

		//
		// Destruktor
		//
		///////////////////////////////////////////////////////////
		~string();

		// 
		// public Methoden
		//
		///////////////////////////////////////////////////////////
		string lower(void);
		string left(int);
		string right(int);
		string mid(int, int);
		void trim(void);
		int len(void);
		int instr(const char*,int);
		int instr(const string,int);
		char* _char(void);	

		// 
		// Überladene Operatoren
		//
		///////////////////////////////////////////////////////////
		char* operator+(const char*);
		char* operator+(string);
		string& operator=(const char*);
		string& operator=(const string&);
		bool operator==(const string);
		bool operator==(const char*);
		bool operator!=(const string);
		bool operator!=(const char*);	
	};
};//namespace

#endif

#include"parsx.h"

namespace prsx
{
        // char in int
	int convert(char *c) 
	{ 
		int counter=prsx::len(c);
		int result=0;
		int summ  =0;
		int pot   =1;

		while(counter>0)
		{
			counter--;
			summ = ((prsx::len(c))-1 - counter);

			for(int i=0; i<summ; i++)
				pot=pot*10;

			summ = pot * (((c[counter] >= '0' && c[counter] <= '9') ? c[counter]-'0' : -1));

			result = result + summ;
			pot=1;
		}
		return result; 
	} 

	int len(const char* st)
	{
		int counter=0;
		while (st[counter])
		{
			counter++;
		}
		return counter;
	}

	//
	//	Konstruktor / Copykonstruktor
	//
	///////////////////////////////////////////////////////////
	string::string()
	{
		m_string=0L;
	}

	string::string(const char* string)
	{	
		m_string=0L;
		cpychar(string);
	}

	string::string(const string& prx)
	{
		m_string=0L;
		cpychar(prx.m_string);
	}

	//
	//	Destruktor
	//
	///////////////////////////////////////////////////////////
	string::~string()
	{
		if(m_string)
			delete[] m_string;
	}

	//
	//	Überladene Operatoren
	//
	///////////////////////////////////////////////////////////
	char*
	string::operator+(const char* t)
	{
		char* temp_char=new char[this->len()+prsx::len(t)];

		int counter=0;

		while(counter<=this->len())
		{ 
			temp_char[counter]=m_string[counter]; 
			counter++;
		}

		counter--;
		while(counter<=this->len()+prsx::len(t))
		{ 
			temp_char[counter]=t[counter-this->len()]; 
			counter++;
		}

		temp_char[counter]='\0';

		return temp_char;
	}

	char*
	string::operator+(string str)
	{
		char* t = str._char();

		char* temp_char=new char[this->len()+prsx::len(t)];

		int counter=0;

		while(counter<=this->len())
		{ 
			temp_char[counter]=m_string[counter]; 
			counter++;
		}

		counter--;
		while(counter<=this->len()+prsx::len(t))
		{ 
			temp_char[counter]=t[counter-this->len()]; 
			counter++;
		}

		temp_char[counter]='\0';

		return temp_char;
	}

	string& 
	string::operator=(const char* t)
	{	
		cpychar(t);
		return *this;
	}

	string& 
	string::operator=(const string& prx)
	{
		if(this==&prx)
			return *this;

		cpychar(prx.m_string);
		return *this;
	}

	bool 
	string::operator==(string prx)
	{
		if(this==&prx)
			return true;
		if (len()!=prx.len())
			return false; 
		if(instr(prx._char(),0)==0)return false; 
			return true;
	}

	bool 
	string::operator==(const char* t)
	{
		if(this->len()!=prsx::len(t))
			return false;
		if(instr(t,0)==0)
			return false; 
		return true;
	}

	bool 
	string::operator!=(string prx)
	{
		if (len()!=prx.len())
			return true; 
		if(instr(prx._char(),0)==0)return true; 
			return false;
	}

	bool 
	string::operator!=(const char* t)
	{
		if (len()!=prsx::len(t))
			return true; 
		if(instr(t,0)==0)
			return true; 
		return false;
	}

	//
	//	private Methoden
	///////////////////////////////////////////////////////////

	//
	//	Kopiert str in m_string
	//
	void
	string::cpychar(const char* str)
	{
		delete[] m_string;

		m_string  = new char[prsx::len(str)+1];
		m_counter = 0;

		while(str[m_counter]!='\0')
		{
			m_string[m_counter]=str[m_counter];
			m_counter++;
		}

		m_string[m_counter]='\0';
	}

	//
	//	public Methoden
	//
	///////////////////////////////////////////////////////////

	//
	//	Länge von m_string
	//
	int 
	string::len()
	{
		int counter=0;
		while(m_string[counter])
		{
			counter++;
		}
		return counter;
	}

	//
	//	a Zeilen von links
	//
	string
	string::left(int a)
	{
		char* temp_char = new char[a+1];
		int counter=0;
		a--;
		while(counter<=a)
		{ 
			temp_char[counter]=m_string[counter]; 
			counter++;
		} 
		temp_char[counter]='\0';
		string st=temp_char;

		delete[] temp_char;
		return st;
	}

	//
	//	a Zeilen von rechts
	//
	string
	string::right(int a)
	{
		int counter=0; 
		int l=this->len()-a;
		char* temp_char= new char[a+1];
		while(m_string[l+counter])
		{
			temp_char[counter]=m_string[l+counter];
			counter++;
		}
		temp_char[counter]='\0';
		string temp_parsx=temp_char;

		delete[] temp_char;
		return temp_parsx;
	}

	//
	//	a Zeilen ab b
	//
	string
	string::mid(int a, int b)
	{
		int counter=0;
		char* temp_char=new char[a+1];
		a--;
		while(counter<=a)
		{
			temp_char[counter]=m_string[counter+b-1];
			counter++;
		}
		temp_char[counter]='\0';
		string temp_parsx=temp_char;

		delete[] temp_char;
		return temp_parsx;
	}

	//
	//	ist t ab a in parsx?
	//
	int 
	string::instr(const char* t, int a)
	{
		int	l = prsx::len(t);
		int counter = 0;

		if(a<0)
			return 0;

		while(m_string[a])
		{
			while(t[counter])
			{
				if(t[counter]!=m_string[a+counter]) break;
				counter++;
				if(counter==l) return a+1;
			}
			counter=0;
			a++;
		}
		return 0;
	}

	//
	//	ist parsx ab a in parsx
	//
	int 
	string::instr(string st, int a)
	{
		char* temp_char=st._char();
		int	l = prsx::len(temp_char); 
		int counter = 0;

		if(a<0) 
			return 0;

		while(m_string[a])
		{
			while(temp_char[counter])
			{
				if(temp_char[counter]!=m_string[a+counter]) 
					break;
				counter++;
				if(counter==l) return a+1;
			}
			counter=0;
			a++;
		}
	return 0;
	}

	//
	//	in Kleinbuchstaben umwandeln
	//
	string
	string::lower(void)
	{
		int counter=0;

		while(m_string[counter])
		{
			if(m_string[counter]>64 && m_string[counter]<91)
				m_string[counter]=m_string[counter]+32;
			else
				m_string[counter]=m_string[counter];
			counter++;
		}

		return m_string;
	}

	//
	// Leerzeielen links und rechts löschen
	//
	void
	string::trim(void)
	{
		int front=0;
		while(m_string[front] && m_string[front]==' ')
		{
			front++;
		}

		int back=len()-1;
		while(m_string[back] && m_string[back]==' ')
		{
			back--;
		}
		back=len()-back;

		int a=len()-(front+back);

		char* temp = new char[a+1];

		int counter=0;
		while(counter<=a)
		{ 
			temp[counter]=m_string[counter+front]; 
			counter++;
		} 

		temp[counter]='\0';

		cpychar(temp);
	}

	//
	//	char* zurückgeben
	//
	char*
	string::_char(void)
	{
		return m_string;
	}
};//namespace

[hustbaer]

Örksel. Also nur was mir so auf die Schnelle auffällt...

1. "m_counter" sollte besser "m_length" oder "m_size" heissen, oder ganz verschwinden, da es eh nicht verwendet wird
2. der Code ist an einigen Stellen nicht Exception Safe, das solltest du fixen
3. was du im Operator "+" aufführst ist Schwachsinn, ein temporäres Array mit "new" anlegen und anstatt es wieder zu löschen gibst du nen Zeiger darauf zurück???
4. die "convert" Funktion hat einen besseren Namen verdient, ist SEHR schräg implementiert, und ignoriert einfach alles was nicht 0-9 ist, zählt es aber (von der stellenwertigkeit) trotzdem als stelle??? (einfacher gesagt: interpretiert alles was nicht 1-9 ist als 0)
5. die parameter der "mid" Funktion sind gegenüber dem was man als quasi-standard erwartet vertauscht (mid ist fast immer "mid(anfang, länge)"), und hier haben die Parameter einen besseren Namen verdient ("a" und "b" sind nicht wirklich aussagekräftige Namen in diesem Zusammenhang)
6. du hast zumindest an einer Stelle ein memory leak, nämlich in der "trim" Funktion

Ein paar Tips:
* "if(p) delete [] p;" kannst du durch bloss "delete [] p;" ersetzen, ein delete auf einen 0 Pointer ist OK und tut einfach nix.
* mach dir eine "swap" Funktion die einfach bloss alle Member tauscht, so dass du schnell 2 Strings gegeneinander tauschen kannst. Dazu machst du dann noch einen Konstruktor dem du einfach nur die Länge des Strings mitgibst, der halt passend viel Speicher anfordert. Damit kannst du viele Funktionen viel einfacher implementieren, nach dem Schema:

void string::append(char c)
{
    // alloc temp buffer
    string temp(m_length + 2); // NOTE: m_length ist the size of the string WITHOUT the terminating zero character, so we need m_length + 2 for the new string

    // copy contents (use memcpy if you don't mind using library functions!)
    for(size_t i = 0; i < m_length; i++)
        temp.m_string[i] = m_string[i];

    // append char & terminate new string
    temp.m_string[m_length] = c;
    temp.m_string[m_length + 1] = 0;

    // swap this vs. temp
    swap(temp);
    // temp now controls this string's old buffer and will release it properly
    // when it goes out of scope, which is just about ... now :)
}

[jeans]

Da habe ich ja noch einiges Nachzubessern. Danke hustbaer.