Problem z przeciążonymi operatorami '=' i '+'

0

Hej. Mam problem z przeciążonymi operatorami '=' i '+'. Deklaruję klasę cTekst służącą do manipulacji na ciągach znaków. Zawiera ona dwie dane wewnętrzne: char* mpTekst i unsigned int mDlugosc. Pierwsza dana będzie przechowywać tablicę znaków na stercie, druga zawierać ilość znaków w tej tablicy. Mam utworzone trzy konstruktory - cTekst(), cTekst(const char*, unsigned int) i kopiujący cTekst(cTekst&). Przeciążam operatory '+' i '='. W pojedynkę działają dobrze, program kompiluje się bez błędów. Gdy jednak próbuję zastosować zapis a = b + c (a, b, c - obiekty cTekst) kompilator krzyczy następujący komunikat: "no match for 'operator=' in 'a = cTekst::operator+(cTekst&)(((cTekst&)(&c)))' candidates are: cTekst& cTekst::operator=(cTekst&)". Poniżej zamieszczam nagłówek i część implementacji tej klasy:

// cTekst.hpp
class cTekst
{
    public:
        cTekst();
        cTekst(const char* tekst, unsigned int dlugosc);
        cTekst(cTekst& rhs);
        ~cTekst();
		
        // metody znakowe
        bool PobierzZnak(unsigned int n, char& bufor);    // zapisuje znak n ciągu do podanego bufora
        bool ZapiszZnak(unsigned int n, const char znak); // nadpisuje n-ty znak podaną wartością
	
        // długość
        unsigned int PobierzDlugosc() const; // zwraca długość
		
		
        // operatory
        cTekst& operator= (cTekst& rhs);
        cTekst operator+ (cTekst& rhs);
		
		
    private:	
        char* mpTekst;
        unsigned int mDlugosc; // długość tekstu bez znaku końca '\0'
	
};

i najistotniejsze implementacje:

// konstruktor
cTekst::cTekst() : mpTekst(0), mDlugosc(0)
{
	
}


// przeciążony kontruktor
cTekst::cTekst(const char* tekst, unsigned int dlugosc) : mpTekst(0), mDlugosc(0)
{
    mpTekst = new char [dlugosc + 1];
    for ( int i = 0; i < dlugosc; i++ )
        mpTekst[i] = tekst[i];
		
    mpTekst[dlugosc] = '\0';
	
    mDlugosc = dlugosc;
}

// konst. kopiujący
cTekst::cTekst(cTekst& rhs)
{
    mpTekst = new char [rhs.PobierzDlugosc() + 1]; // plus jedno miejsce na znak na pusty
    for ( unsigned int i = 0; i < rhs.PobierzDlugosc(); i++ )
    {
        char bufor;
        rhs.PobierzZnak(i, bufor);
        mpTekst[i] = bufor;
    }
	 
    // kończymy pustym znakiem
    mpTekst[rhs.PobierzDlugosc()] = '\0';
	
    mDlugosc = rhs.PobierzDlugosc();
}


// operator =
cTekst& cTekst::operator= (cTekst& rhs)
{
    // jeśli adres aktualnego obiektu jest identyczny z adresem obiektu do
    // którego odnosi się referencja rhs, czyli przypisujemy ten sam obiekt do
    // siebie, po prostu zwracamy aktualny obiekt
    if ( this == &rhs )
        return *this;
	
    __TekstBezpiecznieUsunTablice(mpTekst);    // usuwamy 
    mpTekst = new char [rhs.PobierzDlugosc() + 1];    // alokujemy na nowo pamięć
    // przepisujemy znaki
    for ( int i = 0; i < rhs.PobierzDlugosc(); i++ )
    {
        char bufor;
        rhs.PobierzZnak(i, bufor);
        mpTekst[i] = bufor;
    }
	
    // kończymy psutym znakiem
    mpTekst[rhs.PobierzDlugosc()] = '\0';
	
    // przepisujemy rozmiar
    mDlugosc = rhs.PobierzDlugosc();
	
    return *this;
}


// operator+
cTekst cTekst::operator+ (cTekst& rhs)
{
    unsigned int dlWyniku;
    char* nowyTekst = 0;
	
    // długość sumy ciągów znaków jest równa sumie tych długości :)
    dlWyniku = mDlugosc + rhs.PobierzDlugosc();
    // aklokujemy nową tablicę znaków i przepisujemy do niej ciąg z tego (this)
    // obiektu i rhs
    nowyTekst = new char [dlWyniku + 1];
    for ( int i = 0; i < mDlugosc; i++ )
        nowyTekst[i] = mpTekst[i];
    for ( int i = 0; i < rhs.PobierzDlugosc(); i++ )
    {
        char bufor;
        rhs.PobierzZnak(i, bufor);
        nowyTekst[i + mDlugosc] = bufor;
    }
	
    // dodajemy na koniec zero (just in case...)
    nowyTekst[dlWyniku] = '\0';
	
    // tworzymy obiekt cTekst z tekstem uzyskanym przez proces dodawania
    cTekst wynik(nowyTekst, dlWyniku);
	
    // usuwamy zbędny bufor
    delete [] nowyTekst;
    nowyTekst = 0;
	
    // zwracamy wynik
    return wynik;
}


Oki, jak przebrnęliście przez powyższą sieczkę, to spox. I teraz kiedy wywala error:

cTekst proba1("Ten tekst będzie", 16);
cTekst proba2("kiedyś jednością!", 17);
cTekst proba3;

proba3 = proba1 + proba2;   // tu wywala

Zdaję sobię sprawę, dlaczego kompilator zgłasza błąd - operator= pobiera jako argument referencję do cTekst a nie sam obiekt. A operator+ zwraca cTekst. Owszem, mogę zapisać to tak: cTekst& operator+ (cTekst&) i wartość zwracana zgadzałaby się z argumentem operatora =. I tak bym zrobił, gdyby nie potrzeba utworzenia nowego obiektu wewnątrz operatora +. Do lokalnej zmiennej referencji zwracać nie wolno, to wiem. Można niby utworzyć ten obiekt na stercie, ale późniejsze zwalnianie pamięci wprowadziłoby za dużo komplikacji. A powinno być prosto, zrozumiale :). Nie wiem jak ten problem rozwiązać, byłbym serdecznie wdzięczny za każdą pomoc. Btw. nie testowałem jeszcze poprawności działania operatora + (z przyczyn banalnych - nie kompiluje się), więc mogą być w nim błędy. Z tym ort! poradzę, no prob, potrzebuję tylko odpowiedzi na pytanie: jak poprawnie łączyć operatory = i +. Dzięki z góry!

ps. jak znajdziecie jakieś inne niedopatrzenia, czy znacie lepszy sposób zapisu etc. to piszcie również! thx.

0

Może to:

cTekst& operator= (const cTekst& rhs);
cTekst operator+ (const cTekst& rhs);

:-)

cTekst::cTekst(const char* tekst, unsigned int dlugosc) : mpTekst(0), mDlugosc(0)
{
    mpTekst = new char [dlugosc + 1];
    for ( int i = 0; i < dlugosc; i++ )
        mpTekst[i] = tekst[i];
                
    mpTekst[dlugosc] = '\0';
        
    mDlugosc = dlugosc;
}

Parametr dlugosc jest zbędny -> strlen. No i te kopiowanie -> strcpy.

0
class cTekst
{
    public:
        cTekst();
        cTekst(const char* tekst, unsigned int dlugosc);
        cTekst(cTekst& rhs);
        ~cTekst(){};

        // metody znakowe
        bool PobierzZnak(unsigned int n, char& bufor){};    // zapisuje znak n ciągu do podanego bufora
        bool ZapiszZnak(unsigned int n, const char znak){}; // nadpisuje n-ty znak podaną wartością

        // długość
        unsigned int PobierzDlugosc() const{}; // zwraca długość


        // operatory
        cTekst& operator= (cTekst& rhs);
        cTekst& operator+ (cTekst& rhs);


    private:
        char* mpTekst;
        unsigned int mDlugosc; // długość tekstu bez znaku końca '\0'

};





// konstruktor
cTekst::cTekst() : mpTekst(0), mDlugosc(0)
{

}


// przeciążony kontruktor
cTekst::cTekst(const char* tekst, unsigned int dlugosc) : mpTekst(0), mDlugosc(0)
{
    mpTekst = new char [dlugosc + 1];
    for ( int i = 0; i < dlugosc; i++ )
        mpTekst[i] = tekst[i];

    mpTekst[dlugosc] = '\0';

    mDlugosc = dlugosc;
}

// konst. kopiujący
cTekst::cTekst(cTekst& rhs)
{
    mpTekst = new char [rhs.PobierzDlugosc() + 1]; // plus jedno miejsce na znak na pusty
    for ( unsigned int i = 0; i < rhs.PobierzDlugosc(); i++ )
    {
        char bufor;
        rhs.PobierzZnak(i, bufor);
        mpTekst[i] = bufor;
    }

    // kończymy pustym znakiem
    mpTekst[rhs.PobierzDlugosc()] = '\0';

    mDlugosc = rhs.PobierzDlugosc();
}


// operator =
cTekst& cTekst::operator= (cTekst& rhs)
{
    // jeśli adres aktualnego obiektu jest identyczny z adresem obiektu do
    // którego odnosi się referencja rhs, czyli przypisujemy ten sam obiekt do
    // siebie, po prostu zwracamy aktualny obiekt
    if ( this == &rhs )
        return *this;

    
    mpTekst = new char [rhs.PobierzDlugosc() + 1];    // alokujemy na nowo pamięć
    // przepisujemy znaki
    for ( int i = 0; i < rhs.PobierzDlugosc(); i++ )
    {
        char bufor;
        rhs.PobierzZnak(i, bufor);
        mpTekst[i] = bufor;
    }

    // kończymy psutym znakiem
    mpTekst[rhs.PobierzDlugosc()] = '\0';

    // przepisujemy rozmiar
    mDlugosc = rhs.PobierzDlugosc();

    return *this;
}


// operator+
cTekst& cTekst::operator+ (cTekst& rhs)
{
    unsigned int dlWyniku;
    char* nowyTekst = 0;

    // długość sumy ciągów znaków jest równa sumie tych długości :)
    dlWyniku = mDlugosc + rhs.PobierzDlugosc();
    // aklokujemy nową tablicę znaków i przepisujemy do niej ciąg z tego (this)
    // obiektu i rhs
    nowyTekst = new char [dlWyniku + 1];
    for ( int i = 0; i < mDlugosc; i++ )
        nowyTekst[i] = mpTekst[i];
    for ( int i = 0; i < rhs.PobierzDlugosc(); i++ )
    {
        char bufor;
        rhs.PobierzZnak(i, bufor);
        nowyTekst[i + mDlugosc] = bufor;
    }

    // dodajemy na koniec zero (just in case...)
    nowyTekst[dlWyniku] = '\0';

    // tworzymy obiekt cTekst z tekstem uzyskanym przez proces dodawania
    cTekst wynik(nowyTekst, dlWyniku);

    // usuwamy zbędny bufor
    delete [] nowyTekst;
    nowyTekst = 0;

    // zwracamy wynik
    return wynik;
}

int main()
{
cTekst a,b,c;
a = b+c;
}

Zawsze mozna tak :) i tez bedzie dzialac. Nie wiem czemu ale bedzie. Tez mi sie wydaje ze nie powinno dzialac a jednak. Nie potraffie tego wytlumaczyc bo wraz z zakonczeniem funkcji obiekt ten powinien byc likwidowany, ale zapytaj sie kogos kto sie bardziej zna na tym :). Albo dla pewnosci utworzyc zmienna wynik jako statyczna i po sprawie.

// wydaje mi się, że łatwiej czyta się kod, jeśli jest on pokolorowany, używaj więc odpowiednich znaczników! - Q

0
Hibo napisał(a)
cTekst& cTekst::operator+ (cTekst& rhs)
{
   ...
  
    cTekst wynik(nowyTekst, dlWyniku);

   ...
   
    return wynik;
}

Zawsze mozna tak :) i tez bedzie dzialac.

To, że działa to czytsty przypadek ;) To jest zła konstrukcja - zwraca referencję na obiekt, który (teoretycznie) nie istnieje - bo jest obiektem lokalnym. Przy wyjściu z operator+ zostanie wywołany destruktor obiektu wynik, a ten zwolni pamięć wskazywaną przez mpTekst.

0

A to jest dość charakterystyczne zagadnienie, jak się przyglądnąć dokładnie, bo w C++ funkcje:

Klasa funkcja() {
   Klasa xx;
   // operacje
   return xx;
   }
// ...
funkcja()

są zamieniane często na coś takiego:

void funkcja(Klasa& __result) {
   __result.X::X();
   // operacje na __result
   return;
   }
// ...
{
   Klasa __result; // tutaj NIE jest wywoływany konstruktor!
   funkcja(__result);
}

To się nazywa optymalizacją NRV i pozwala uniknąć niepotrzebnego kopiowania. Więc po pierwsze: olać to, że zwracanie przez wartość jest nieoptymalne. Już kompilator się postara o to, żeby było. Po drugie: długość życia zmiennej __result jest nieznana - tzn. nie wiadomo, gdzie dokładnie zostaną wstawione klamry {}, więc nie próbować nawet kombinować z wykorzystaniem tego mechanizmu w jakiś chakerski sposób.

0

Co nie zmienia faktu, że trzeba pamiętać o regule zasięgu zmiennych, bo ta bez względu na to w jaki sposób kompilator zoptymalizuje kod, będzie taka sama.

0

Oczywiście. Nawet jeśli zmienne są tworzone w pamięci kiedyś-tam i jakoś-tam, to moment wywołania konstruktorów i destruktorów jest ściśle określony przez standard i zasięg bloków w kodzie źródłowym (a nie w zoptymalizowanym). Dlatego napisałem, żeby nie kombinować z wykorzystywaniem tej cechy ;)

0

Wiem ze obiekt bedzie likwidowany, dlatego potem dopisalem ze mozna utworzyc znienna statyczny i juz :). A odbiegajac od tematu moze mi ktos cos wyjacnic. Jesli zakladajac ze mamy program i tylko on moze ort! z wyznaczonego segmentu pamieci, tak ze inne ort! nie moga go ruszyc nawet gdy nie jest uzywany to chyba funkcja ktora by cos zwrocila przez referencje i by zostalo Od razu to zapisane gdzies to by bledu nie bylo no. Przeciez destruktory zwalniaja tylko pamiec, pozostawiajac wynik bez zmian. Co stego ze pod danym adresem nie istnieje zadna zmienna, ale wynik nie ulega zmianie. Moze mi ktos powiedziec czy mam racje czy sie myle?

0

Są destruktory które robią więcej niż tylko zwalniają pamięć, gwarantuję. Jeśli znasz treść destruktora i zachowanie kompilatora, to możesz ryzykować. Ale rozwiązanie będzie niezgodne ze wszystkimi paradygmatami, jakie istnieją. Taki kod będzie kruchy jak pajęczyna, a błędy będą mogły się pojawić w najmniej oczekiwanym momencie.

0

A odbiegajac od tematu moze mi ktos cos wyjacnic. Jesli zakladajac ze mamy program i tylko on moze kazystac z wyznaczonego segmentu pamieci, tak ze inne aplikazje nie moga go ruszyc nawet gdy nie jest uzywany to chyba funkcja ktora by cos zwrocila przez referencje i by zostalo Od razu to zapisane gdzies to by bledu nie bylo no.

No i prawdopodobnie dla tego u Ciebie wersja, którą zaproponowałeś działała, chociaż była błędna. Nie wiem jakiego kompilatora używasz, ale w VC z pełną diagnostyką ten numer by nie przeszedł.

Co stego ze pod danym adresem nie istnieje zadna zmienna, ale wynik nie ulega zmianie. Moze mi ktos powiedziec czy mam racje czy sie myle?

Tak, tylko, że nie masz już prawa dostępu do tego skrawka pamięci (mówię o stercie).

0
0x666 napisał(a)

Co stego ze pod danym adresem nie istnieje zadna zmienna, ale wynik nie ulega zmianie. Moze mi ktos powiedziec czy mam racje czy sie myle?

Tak, tylko, że nie masz już prawa dostępu do tego skrawka pamięci (mówię o stercie).

Do tego nie masz ZADNEJ gwarancji, ze ten wynik nie ulegnie zmianie.

0

Noo, jakąś gwarancję uzyskać można - przeglądasz wygenerowany kod w asemblerze i patrzysz, co ci kompilator wygenerował ;]

0
Ranides napisał(a)

Noo, jakąś gwarancję uzyskać można - przeglądasz wygenerowany kod w asemblerze i patrzysz, co ci kompilator wygenerował ;]

Taaa, dzieki za taka gwarancje :P

0

Ok dzieki za wyjasnienie. A co do kodu ktory zaproponowales. Troche ciezko to odniesc do operatora+. Bo takie wywolanie a.operator+(b, wynik); nie zadowoli kazdego ;/. Nie lepiej utworzyc zmienna statyczna w klasie i to ona by przetrzymywala wyniki dla wszystkich przeciazanych operatorow? A i jak mi wyjasnicie ze w ort! stanadard jest taki kod:

//***************************************************
// Program z paragrafu   19.8.1 (str 811)
//***************************************************

// Sprawdzony na Linuksie, kompilator: GNU gcc version 3.3.3 (SuSE Linux)
// Sprawdzony na Windows XP,  kompilator: Microsoft Visual C++ 6.0
 
#include <iostream>
using namespace std ;


/////////////////////////////////////////////////////////
class wektorek {
public :
     float x, y, z ;
     // --- konstruktor
     wektorek(float xp = 0, float yp = 0, float zp = 0 )
                    : x(xp), y(yp), z(zp)                  //
     { /* cialo puste */ } ;

     // ... inne funkcje skladowe
} ;
/////////////////////////////////////////////////////////
wektorek operator*(wektorek kopia, float liczba )     //

{
wektorek rezultat ;

     rezultat.x = kopia.x * liczba ;
     rezultat.y = kopia.y * liczba ;
     rezultat.z = kopia.z * liczba ;
     return rezultat ;
}
/*******************************************************/
void pokaz(wektorek www) ;          // deklaracja
/*******************************************************/
int main()
{
wektorek     a(1,1,1) ,
          b(-15, -100, +1) ,
          c ;

          c = a * 6.66 ;                               //
          pokaz(c) ;

          c = (b * -1.0)*8 ;
          pokaz(c) ;
            system("pause");
}
/*******************************************************/
void pokaz(wektorek www)
{
      cout << " x = " <<  www.x
           << " y = " <<  www.y
           << " z = " <<  www.z  << endl ;
}


/************************************************************


************************************************************/

I wszystko za kazdym razem dziala poprawnie. Czyzby w ksiazce ort! w Polsce(jesli chodzi o c+) byl taki blad?

//DOBRA ort! NIE MYSLE DZISIAJ. JUZ WSZYSTKO WIEM. PO PROSTU WSTALEM O 3 W NOCY I JESTEM NIE TEGES :).

// o 3 to informatyk kładzie się spać, a nie wstaje! ;) - Q

0

Nie, no ja czasem za mądrze chyba bełkoczę, bo mnie ludzie olewają ;) Zapomnij o poradach Hibo, bo są lewe i skorzystaj z tego, co ci 0x666 napisał zaraz pod pierwszym postem. Czyli:

  1. operator+ masz w porządku, niech tworzy sobie nowy obiekt i niech go zwraca przez wartość. Nie ruszaj! W C++ to jest dobra i EFEKTYWNA metoda.

  2. operator porównania pobiera referencję do rhs, ale go nie modyfikuje, więc zmień jego deklarację na:

cTekst& operator= (const cTekst& rhs);

Wtedy będzie mógł pobierać argumenty które zostały zwrócone przez inne funkcje jako wartość.

  1. Funkcja PobierzZnak nie modyfikuje obiektu, więc dodaj do jej deklaracji również modyfikator const
bool PobierzZnak(unsigned int n, char& bufor) const;

I wszystko będzie działać poprawnie, szybko etc. Składowe statyczne zwracane przez referencje w pewnych sytuacjach będą sprawiać problemy i są niepotrzebną komplikacją. Poza tym są nieeleganckie ;)

0

Nie lepiej utworzyc zmienna statyczna w klasie i to ona by przetrzymywala wyniki dla wszystkich przeciazanych operatorow?

Zrozumiesz, że to jest błędne myślenie jak zaczniesz pisać programy wielowątkowe.

Czyzby w ksiazce najlepiszej w Polsce(jesli chodzi o c+) byl taki blad?

Nie wiem, może ślepy jestem, ale nie widzę nigdzie błędów... no może te przekazywanie parametrów przez wartość - błąd projektowy :P

0

Wielkie dzięki za udzielenie porady. Wszystko jest już OK!

1 użytkowników online, w tym zalogowanych: 0, gości: 1