string-klasse

[Pogo]

Hier, das ist vielleicht keine besonders gute oder effiziente Klasse, aber sie funktioniert immerhin! Sie wurde halt von einem Anfänger geschrieben.

[siehe späteren Beitrag]

[Pogo]

Ich weis, ich hätte z.B. die substring Methode effizienter schreiben können. Ich hätte nur ein paar Zeiger inkrementieren oder auch dekrementieren brauchen und das neue Stringobjekt hätte sich den Speicher mit dem Alten teilen können. Aber ich war mir da nicht so sicher. Denn wenn das eine Objekt gelöscht wird, dürfte es Probleme geben. Und ich weis auch, dass ich die ganze Klasse hätte besser machen können und z.B. mit den beliebten template arbeiten können. Aber das ist ja alles im Konjunktiv, denn ich schreibe lieber code den ich kapiere!

[Bashar]

Ich seh haufenweise new, aber nirgends delete. Sehr fragwürdige Empfehlung, IMHO.

[Pogo]

Ach ja, habe noch was vergessen. Da diese Klasse eben noch ziemlich schlecht ist, bitte ich um Verbesserungsvorschläge! Danke!

[Braunstein]

Nun ja, wie man am obigen Code sieht, kann man anscheinend auch eine string-Klasse ohne Speicherverwaltung schreiben.

Du müsstest deinen Speicher schon intern verwalten. So wie du das jetzt machst übergibst du die Verantwortung dem Nutzer. Er muss Speicher vereinbaren und auch wieder löschen. Das geht auf jeden Fall schief.

[ Dieser Beitrag wurde am 07.05.2003 um 17:49 Uhr von Braunstein editiert. ]

[Walli]

Original erstellt von Pogo:
Ach ja, habe noch was vergessen. Da diese Klasse eben noch ziemlich schlecht ist, bitte ich um Verbesserungsvorschläge! Danke!

Vorschlag #1: Memory-Leaks entfernen (also wie Bashar gesagt hat: auch mal deleten).
Vorschlag #2: PreIncrement verwenden (also ++i anstatt i++) wenn PostIncrement unsinnig ist -> siehe FAQ.
Vorschlag #3: Mal so'n paar Sachen auf den ersten Blick korrigiert

bool String::equals(String &string) const
{
    char* value2 = string.toCharPointer();
    char *p1 = value2, *p2 = value; // <- so besser
    bool same = true; // <- wenn du es hier schon auf true setzt sparst du dir viele Male same = true unten
    while(p1[0] != '\0' && p2[0] != '\0')
    {
        if(p1[0] == p2[0])
        {
          ++p1; // <- ++p1 ist hier schneller als p1++
          ++p2;
        }
        else
        {
          same = false;
          break;
        }
    }
    return same;
}

[Pogo]

Oh! Du hast recht. Stimmt, das habe ich ganz vergessen.
Naja, das lässt sich ja leicht "nachimplementieren".

[Walli]

Original erstellt von Braunstein:
Nun ja, wie man am obigen Code sieht, kann man anscheinend auch eine string-Klasse ohne Speicherverwaltung schreiben.

So ganz ohne wird das nicht gehen...
HumeSikkins sprach ausserdem davon, dass man bei einem eigenen OS auch eine eigene Speicherverwaltung braucht...

[Braunstein]

Wie du an seinen Konstruktoren siehst vereinbart er keinen neuen Speicher sondern nutzt den der übergebenden Variablen.
Siehe hier:

String::String(char* string) {

    value = string;
    length = getLength(string);
}

Wenn in diesen Code nicht noch sehr viel fehlt, muss also die Speichervereinbarung und Löschung von der übergebenen Variablen übernommen werden und das bezeichne ich als schlecht designed.

Ciao

[Gerard]

String::String() {

    value = "NULL";
    length = 4;
}

aua

[Shade Of Mine]

[cpp]
bool String::equals(const String &string) const
{
char *p1 = string.toCharPointer(), *p2 = value;
while(*p1 != '\0' && *p2 != '\0') // oder besser while(*p1&&*p2)
{
if(*p1 == *p2)
{
++p1;
++p2;
}
else
{
return false;
}
}
return true;
}[/cpp]

ich finde p[0] ist irgendwie unlogisch. es ist zwar das selbe wie *p - aber man (zumindest ich) erwartet bei p[0] doch irgendwie eine änderung von [0] und nicht vib p..

also laufende zeiger mit *
und über indices mit []

man kanns auch anders herum machen, aber ich denke so ist es die gängige variante.

ich würde nicht zweimal equals implementieren, sondern einfach:
bool String::equals(const String &string) const { return equals(string.toCharPointer(); }
schreiben

du vergisst oft const zu schreiben...

for (int i = 0; i < getLength(buffer); i++) { buffer[i] = p[c++]; c2++; }
    buffer[c2] = '\0';

ein memcpy wäre besser

und getLength(const char*) ist sowieso blödsinn - dafür gibt es strlen()

if (endIndex > getLength() || beginIndex > getLength() || beginIndex > endIndex) return String("ERROR");

blödsinn...
ein ERROR string hat keinen sinn.
entweder du machst n assert oder wirfst ne exception - das hier ist ein logik fehler!

if (p[0] >= 97 && p[0] <= 122) p[0] -= 32;

verwende doch tolower/toupper

getLength(string.toCharPointer());

wie wäre es mit string.getLength() ?

du hast einige sehr verwirrende funktionen - du solltest in denen den code vielleicht kommentieren und/oder lesbarer schreiben...

du brauchst keine statusvariablen um
return same;
zu schreiben zu können - du kannst ja statt same=false; einfach return false; schreiben

du solltest keine memory leaks haben...

und die klasse sollte den speicher selber verwalten und nicht der user

du solltest nicht sooft den selben code schreiben - probiere eine funktion mit hilfe einer anderen zu implementieren

das ist mir beim drüberfliegen aufgefallen

[Gerard]

Shade Of Mine du vergisst das er ein eigenes os macht, klar wäre ein tolower besser aber soll er die komplete cstdlib nach programmieren?

[Shade Of Mine]

Original erstellt von Dimah:
Shade Of Mine du vergisst das er ein eigenes os macht, klar wäre ein tolower besser aber soll er die komplete cstdlib nach programmieren?

jein.

ein tolower/toupper braucht er sowieso!
ein strlen auch.

sicher muss er nicht die ganze C Library nachprogrammieren - aber die wichtigsten sachen...

oder willst du immer wenn du tolower brauchst eine schleife machen in der du mit ascii code rumspielst?

gerade wenn man ein OS schreibt - sollte man ordentliche bibliotheksfunktionen bereit stellen - sonst hat man doch keinen spaß daran, programme für das OS zu schreiben...

[Lars]

Original erstellt von Dimah:
Shade Of Mine du vergisst das er ein eigenes os macht, klar wäre ein tolower besser aber soll er die komplete cstdlib nach programmieren?

ja. nur so lassen sich auch weiterhin vernünftig programme portieren.

[Pogo]

Danke für die vielen "Verbesserungsvorschläge".
Also mal ein paar Worte zu meiner Verteidigung:

Wie ich ja schon sagte bin ich ein Anfänger. Ich wollte einfach mal ein bisschen rumprobieren um ein bisschen Erfahrung im Umgang mit C++ zu bekommen.
Also habe ich zu Übungszwecken alle Methoden selbst geschrieben. Ich wollte eben meine eigene Klasse ohne includes schreiben.

So und nun ´ne Frage:

char* str1 = "String";
char* str2 = "...";

Wenn ich nun

str2 = str1;

schreibe, was passiert dann? Wird der Wert von str1 str2 zugewiesen oder zeigt hiernach str2 einfach nur auf str1?

Und noch eine Frage zu Refenzen:
Also im Buch stand dass ich auf keinen Fall NULL-Referenzen fabrizieren solle.
Aber das "darf" ich doch, so lange ich diese Referenz nicht mehr verwende ist das doch egal, oder?

[ Dieser Beitrag wurde am 08.05.2003 um 17:25 Uhr von Pogo editiert. ]

[Pogo]

Noch was:

Was macht die Methode tolower, die ich schon in der lib finden kann?
Gibt es da einen so großen Unterschied zu meiner Version?
Sicherlich wird diese nicht ganz so stümperhaft wie meine sein, aber sie wird doch im Prinzip das gleiche machen, oder? Und um irgendwelche bestimmten Zeichencodes brauch eich mir ja auch keine Sorgen machen, da ja eigentlich die ersten 256 Zeichen der ganzen Zeichencodes übereinstimmen.

@ "Shadow of Mine"

Welche Methoden findest du verwirrend?

[Shade Of Mine]

Original erstellt von Pogo:
**@ "Shadow of Mine"

Welche Methoden findest du verwirrend? :)**

Shade, bitte

zB regionMatches (viel zu lange), concat (irgendwie so viele while schleifen ) und replace (soviele ifs).

[neverbeenhere]

und noch was:

String::String(char* string) {

value = string;
length = getLength(string);
}

was, wenn der pointer string nach dem aufruf dieses ctors deleted wird?? dann ist value ungültig! also muss man erst noch speicher reservieren, und zwar length+1 byte und dann strcpy&co benutzen. bei mir sähe das dann so aus:

String::String(char* string) {
length = getLength(string);
value = new char[length+1]
strcpy (value, string);
}

naja, wie gesagt, das einzige problem bei meiner str-klasse ist, das es speicher-lecks gibt und wenn ich das beheben will, stürzt er immer ab...
eine eig. speicherverwaltung hab ich natürlich auch, genauso wie viele routinen der cstdlib (oft allerdings mit anderem namen, z.b. string_copy usw. (das is gerade für den anfänger noch verständlicher (auch wenn ich mich schon eone weile nicht mehr als solchen betrachte...)

cu todo

[Pogo]

Ich habe meine Klasse jetzt auch in einigen Bereichen verbessert:

class String {

public:
    String(char* string); // Konstruktor
    String(String &string); // der parameter senseless ist sinnlos und wird nicht gebraucht, muss aber übergeben werden
    String();
    ~String();
    bool equals(String &string) const;
    bool equals(char* string) const; // String vergleichen, beachtet Groß-Kleinschreibung.
    int getLength() const; // Liefert die Länge des instanzeigenen Strings zurück.
    int getLength(char* string) const; // Liefert Länge des übergebenen Strings zurück.
    char charAt(int index) const; // Liefert char am Index index zurück.
    bool equalsIgnoreCase(String &string) const; // String vergleichen ohne Groß-/Kleinschreibung zu beachten.
    bool equalsIgnoreCase(char* string) const; 
    bool regionMatches(bool ignoreCase, int toffset, String &other, int ooffset, int len) const; // ignoreCase = Groß-/Kleinschreibung beachten
                                                                                          // toffset = index ab wann hiesiger String verglichen werden soll
                                                                                          // other = zu vergleichener String
                                                                                          // ooffset = index ab dem anderer (zu vergleichener) String verglichen werden soll
                                                                                          // len = Länge des String der übereinstimmen soll 
    bool regionMatches(bool ignoreCase, int toffset, char* other, int ooffset, int len) const;  
    bool startsWith(String &string) const; // Prüft ob der Anfang des Strings mit "string" übereinstimmt.
    bool startsWith(char* string) const;
    bool endsWith(String &string) const; // Prüft ob das Ende des Strings mit "string" übereinstimmt.
    bool endsWith(char* string) const;
    void concat(String &string); // Die Verkettung dieses Strings mit string.
    void concat(char* string);
    char* toCharPointer(); // Gibt String-Objekt in einem char-Zeiger(char*) zurück.
    void replace(char oldChar, char newChar); // Sucht alle Zeichen oldChar und ersetzt sie durch newChar.
    void replaceCharAt(int index, char newChar);
    void replace(char* oldString, char* newString); // Sucht alle Strings oldString und ersetzt die komplett durch newString, dürfte recht speicherintensiv sein...
    void replace(String &oldString, String &newString);
    int indexOf(String &string, int fromIndex) const; // Erste Position, ab dem Index "index", des in diesem String vollständig enthaltenen Strings "string".
    int indexOf(char* string, int fromIndex) const;
    int lastIndexOf(String &string, int fromIndex) const; // Letzte Position, ab dem Index "index", des in diesem String vollständig enthaltenen Strings "string".
    int lastIndexOf(char* string, int fromIndex) const;
    String substring(int beginIndex) const; // Neuen String ab Index "index" erstellen.
    String substring(int beginIndex, int endIndex) const; // Neuen String von Index "beginIndex" bis zu Index "endIndex" erstellen(endIndex inkl.). 
    void toLowerCase();
    void toUpperCase();
    int numberOf(char thatChar) const;

    void trim(); // Alle Werte am Anfang und am ende des Strings mit dem Wert kleiner oder gleich 32(whitespace-Zeichen) entfernen.
    void trim(char toTrim);

    String operator+(String &string);
    void operator+=(String &string);
    String &operator=(String &string);
    String &operator=(char* string);
private:
    char* value; // Speichert Inhalt.
    int length;
};

String::String(char* string) {

    length = getLength(string);
    value = new char[length+1];
    int i = 0;
    while (i < length) value[i] = string[i++];
    value[i] = '\0';
}

String::String(String &string) {

    length = string.getLength();
    value = new char[length+1];
    int i = 0;
    char* str = string.toCharPointer();
    while (i < length) value[i] = str[i++];
    value[i] = '\0';
}

String::String() {

    length = 0;
    value = new char[1];
    value[0] = '\0';
}

String::~String() {

    delete [] value;
}

bool String::equals(String &string) const {

    return equals(string.toCharPointer());
}

bool String::equals(char* string) const {

    char *p1, *p2;
    p1 = string;
    p2 = value;
    while (p1[0] != '\0' && p2[0] != '\0') {
        if (p1[0] == p2[0]) {
            ++p1;
            ++p2;
        } else {
            return false;
        }
    }
    return true;
}

int String::getLength() const {

    return getLength(value);
}

int String::getLength(char* string) const {

    char *p = string;
    int c = 0;
    while (p[0] != '\0') { ++p; ++c; }
    return c;
}

bool String::equalsIgnoreCase(String &string) const {

    return equalsIgnoreCase(string.toCharPointer());
}

bool String::equalsIgnoreCase(char* string) const {

    char *p1, *p2;
    p1 = string;
    p2 = value;
    while (p1[0] != '\0' && p2[0] != '\0') {
        if (p1[0] == p2[0]) {
            ++p1;
            ++p2;
        } else if (p2[0] <= 90 && p2[0] >= 65) {
            if (p1[0] == p2[0] + 32) {
                ++p1;
                ++p2;
            } else {
                return false;
            }
        } else if (p2[0] >= 97 && p2[0] <= 122) {
            if (p1[0] == p2[0] -32) {
                ++p1;
                ++p2;
            } else {
                return false;
            }
        } else {
            return false;
        }
    }
    return true;
}

char String::charAt(int index) const {

    char *p2;
    p2 = value;

    if (getLength() < index) {
        return -1;
    } else {
        return p2[index];
    }
    return -1;
}

bool String::regionMatches(bool ignoreCase, int toffset, String &other, int ooffset, int len) const {

    return regionMatches(ignoreCase, toffset, other.toCharPointer(), ooffset, len);
}

bool String::regionMatches(bool ignoreCase, int toffset, char* other, int ooffset, int len) const {

    if (length < len) {
        return false;
    }
    if (ignoreCase == true) {
        char *p1, *p2;
        p1 = value;
        p2 = other;
        int tlen = 0;
        for (int i = 0; i < toffset; i++) {
            ++p1;
        }
        for (int j = 0; j < ooffset; j++) {
            ++p2;
        }

        while (p1[0] != '\0' && p2[0] != '\0' && tlen < len) {
            ++tlen;
            if (p1[0] == p2[0]) {
                ++p1;
                ++p2;
            } else if (p2[0] <= 90 && p2[0] >= 65) {
                if (p1[0] == p2[0] + 32) {
                    ++p1;
                    ++p2;
                } else {
                    return false;
                }
            } else if (p2[0] >= 97 && p2[0] <= 122) {
                if (p1[0] == p2[0] -32) {
                    ++p1;
                    ++p2;
                } else {
                    return false;
                }
            } else {
                return false;
            }
        }

        return true;
    } else {

        char *p1, *p2;
        p1 = value;
        p2 = other;
        for (int i = 0; i < toffset; i++) {
            ++p1;
        }
        for (int j = 0; j < ooffset; j++) {
            ++p2;
        }
        int tlen = 0;

        while (p1[0] != '\0' && p2[0] != '\0' && tlen < len) {
            ++tlen;
            if (p1[0] == p2[0]) {
                ++p1;
                ++p2;
            } else {
                return false;
            }
        }
        return true;
    }
    return false;
}

bool String::startsWith(String &string) const {

    return startsWith(string.toCharPointer());
}

bool String::startsWith(char* string) const {

    char *p1, *p2;
    p1 = value;
    p2 = string;
    int c = 0;
    int len = getLength(string);
    if (len > length) {
        return false;
    }
    while (p1[0] != '\0' && c < length) {
        if (p1[0] == p2[0]) {
            ++c;
            ++p1;
            ++p2;
        } else {
            return false;
        }
    }

    return true;
}

bool String::endsWith(String &string) const {

    return endsWith(string.toCharPointer());
}

bool String::endsWith(char* string) const {

    int c = 0;
    char *p1, *p2;
    p1 = value;
    p2 = string;

    if (getLength(string) > length - getLength(string)) {
        return false;
    }

    while (p1[0] != '\0') {
        ++p1;

    }
    while (c < length) {
        --p1;
        ++c;
    }

    while (p1[0] != '\0' && p2[0] != '\0') {
        if (p1[0] == p2[0]) {
            ++p1;
            ++p2;
        } else {
            return false;
        }
    }

    return true;
}

void String::concat(String &string) {

    concat(string.toCharPointer());
}

void String::concat(char* string) {

    int len = length + getLength(string);
    char* buffer = new char[len];
    int c = 0;
    while (c < length) buffer[c] = value[c++]; 
    while (c < len) buffer[c] = string[c++];
    buffer[c] = '\0';
    value = buffer;
    delete [] buffer;
}

char* String::toCharPointer() {

    return value;
}

void String::replace(char oldChar, char newChar) {

    char *p = value;

    while (p[0] != '\0') {
        if (p[0] == oldChar) {
            p[0] = newChar;
        }
        ++p;
    }
}

void String::replaceCharAt(int index, char newChar) {

    if (index >= getLength()) {
        return;
    }
    char *p = value;
    p[index] = newChar;
}

void String::replace(char* oldString, char* newString) {

    if (getLength(oldString) == 0) {
        return;
    } else if (getLength(newString) == 0) {
        return;
    }
    for (int i = 0; i < length; i++) {
        if (regionMatches(false, i, oldString, 0, getLength(oldString))) {
            char *p1;
            p1 = value;
            char* buffer = new char[i]; // Alle chars vor i
            int l = length-i-getLength(oldString);
            char* buffer2 = new char[l]; // alle chars ab i und oldString
            if (getLength(buffer) == 0 && l != 0) {
                int c = i + getLength(oldString);
                for (int k = 0; k < l; k++) buffer2[k] = p1[c++];
                buffer2[l] = '\0';
                value = buffer;
                p1 = 0;
                concat(buffer2);
                break;
            } else if (i != 0 && l == 0) {
                for (int j = 0; j < i; j++)  buffer[j] = p1[j];
                buffer[i] = '\0';
                value = buffer;
                p1 = 0;
                concat(newString);
                break;
            } else if (i == 0 && l == 0) {
                p1 = 0;
                value = newString;              
                break;
            } else {
                for (int j = 0; j < i; j++)  buffer[j] = p1[j];
                buffer[i] = '\0';
                int c = i + getLength(oldString);
                for (int k = 0; k < l; k++) buffer2[k] = p1[c++];
                buffer2[l] = '\0';
                value = buffer;
                p1 = 0; 
                concat(newString);
                concat(buffer2);
            }
            delete [] buffer;
            delete [] buffer2;
        }
    }
    length = getLength(value);
}

void String::replace(String &oldString, String &newString) {

    replace(oldString.toCharPointer(), newString.toCharPointer());
}

int String::indexOf(char* string, int fromIndex) const {

    int c = 0;
    for (int i = 0; i < fromIndex; i++) ++c;
    while (c < length) {
        if (regionMatches(false, c, string, 0, getLength(string))) {
            return c;
        } else {
            ++c;
        }
    }
    return -1;
}

int String::indexOf(String &string, int fromIndex) const {

    return indexOf(string.toCharPointer(), fromIndex);
}

int String::lastIndexOf(char* string, int fromIndex) const {

    int c = 0;
    int index = 0;
    for (int i = 0; i < fromIndex; i++) c++;
    while (c < length) {
        if (regionMatches(false, c, string, 0, getLength(string))) {
            index = c;
            ++c;
        } else {
            ++c;
        }
    }
    return index;
}

int String::lastIndexOf(String &string, int fromIndex) const {

    return lastIndexOf(string.toCharPointer(), fromIndex);
}

void String::toLowerCase() {

    char *p = value;
    while (p[0] != '\0') {
        if (p[0] <= 90 && p[0] >= 65) p[0] += 32;
        ++p;
    }
}

void String::toUpperCase() {

    char *p = value;
    while (p[0] != '\0') {
        if (p[0] >= 97 && p[0] <= 122) p[0] -= 32;
        ++p;
    }
}

String String::substring(int beginIndex) const {

    return substring(beginIndex, length);
}

String String::substring(int beginIndex, int endIndex) const {

    if (endIndex > getLength() || beginIndex > getLength() || beginIndex > endIndex) return String(value);
    char* buffer = new char[(endIndex-beginIndex)+2];
    char *p = value;
    int c = 0;
    for (int i = 0; i < beginIndex; i++) p++;
    while (p[0] != '\0' && c <= endIndex-beginIndex) {
        buffer[c++] = p[0];
        ++p;
    }
    buffer[c] = '\0';
    return String(buffer);
}

void String::trim() {

    char *p = value;
    int c = 0;
    while (p[c] <= 32) ++c;
    char* buffer = new char[getLength()-c];
    int c2 = 0;
    for (int i = 0; i < length; i++) { buffer[i] = p[c++]; ++c2; }
    buffer[c2] = '\0';
    --c;
    while (p[c] <= 32) { --c; --c2; }
    buffer[c2] = '\0';
    value = buffer;
    delete [] buffer;   
}

void String::trim(char toTrim) {

    char *p = value;
    int c = 0;
    while (p[c] == toTrim) ++c;
    char* buffer = new char[getLength()-c];
    int c2 = 0;
    int length = getLength(buffer);
    for (int i = 0; i < length; i++) { buffer[i] = p[c++]; ++c2; }
    buffer[c2] = '\0';
    --c; --c;
    while (p[c] == toTrim) { --c; --c2; }
    --c2;
    buffer[c2] = '\0';
    value = buffer;
    delete [] buffer;
}

int String::numberOf(char thatChar) const {

    char *p = value;
    int number = 0;

    while (p[0] != '\0') {
        if (p[0] == thatChar) {
            ++number;
        }
        ++p;
    }
    return number;
}

String& String::operator=(String &string) {

    if (this == &string) return *this;
    delete [] value;
    length = string.getLength();
    int i = 0;
    value = new char[length+1];
    char* str = string.toCharPointer();
    while (i < length) value[i] = str[i++];
    value[i] = '\0';
    return *this;
}

String& String::operator=(char* string) {

    if (&value == &string) return *this;
    delete [] value;
    length = getLength(string);
    int i = 0;
    value = new char[length+1];
    while (i < length) value[i] = string[i++];
    value[i] = '\0';
    return *this;
}

String String::operator+(String &string) {

    String str = *this;
    str.concat(string);
    return str;
}

void String::operator+=(String &string) {

    concat(string);
}

Trotzdem habe ich aber auch ein Problem mit delete. Wie z.B.
in folgender Methode:

void String::trim() {

    char *p = value;
    int c = 0;
    while (p[c] <= 32) ++c;
    char* buffer = new char[getLength()-c];
    int c2 = 0;
    for (int i = 0; i < length; i++) { buffer[i] = p[c++]; ++c2; }
    buffer[c2] = '\0';
    --c;
    while (p[c] <= 32) { --c; --c2; }
    buffer[c2] = '\0';
    value = buffer;
    delete [] buffer;   
}

Dann tritt irgendein Fehler auf und das Programm wird beendet. Ich habe keine Ahnung warum...
Wo liegt da der Fehler? Wenn ich im Destruktor value genau so delete kriege ich ja auch keine Fehlermeldung.

[Pogo]

Ok, ich denke ich weis evtl. wo der Fehler liegt. Irgendwie, klappt das Kopieren von einem String durch meinen =-Operator nicht. Ich habe festgestellt das danach value auf den selben Speicherbereich wie buffer zeigt. Also wurde im Prinzip ja nichst kopiert. Also wie kann ich wirklich kopieren?