[C++] krótki kod do poprawienia

kawafis44
Użytkownik
Użytkownik
Posty: 474
Rejestracja: 22 paź 2007, o 20:55
Płeć: Mężczyzna
Lokalizacja: Gliwice
Podziękował: 416 razy
Pomógł: 2 razy

[C++] krótki kod do poprawienia

Post autor: kawafis44 »

witam!
czy wiecie może, co trzeba poprawić w tym kodzie, żeby on działał?

Kod: Zaznacz cały

#include <cstdlib>
#include <iostream>

using namespace std;

class DynA{
      private:
              int *tab;
              int size;
      public:
             DynA(); //create new array of 10 integers
             ~DynA(); //destructor; delete allocated memory
             int getValue (int index); //gets the value
             void setValue (int index, int newvalue); //sets the value specified by index
             void resize(int new_size);
}

int main(int argc, char *argv[])
{
    DynA da;
    da.resize(100);
    da.set(5,10); //tab[5]=10;
    var = da.get(5); //var = tab[5];
    system("PAUSE");
    return EXIT_SUCCESS;
}
pzdr!
Rafal88K
Użytkownik
Użytkownik
Posty: 311
Rejestracja: 15 mar 2007, o 16:52
Płeć: Mężczyzna
Lokalizacja: Lublin
Podziękował: 28 razy
Pomógł: 54 razy

[C++] krótki kod do poprawienia

Post autor: Rafal88K »

Co ten kod ma robić? Podałeś tylko definicję klasy, a nie ma definicji metod? Do tego w klasie masz metodę getValue i setValue, a niżej wywołujesz metody get i set?

Jak masz dynamiczne zarządzanie pamięci to musisz dla tej klasy napisać konstruktor kopiujący, operator przypisania i destruktor!
Awatar użytkownika
amdfanatyk
Użytkownik
Użytkownik
Posty: 98
Rejestracja: 27 mar 2005, o 14:59
Płeć: Mężczyzna
Lokalizacja: /dev/zero
Podziękował: 9 razy
Pomógł: 7 razy

[C++] krótki kod do poprawienia

Post autor: amdfanatyk »

Dodać średnik na końcu klasy i zdefiniować jej metody bo na razie zostały jedynie zadeklarowane a kompilator nie domyśli się co chcesz, żeby program robił. Po za tym kompilator odróżnia set od setValue od SetValue od SETVALUE od seTvalue od set_value i też weź to pod uwagę.
Rafal88K pisze:Jak masz dynamiczne zarządzanie pamięci to musisz dla tej klasy napisać konstruktor kopiujący, operator przypisania i destruktor!
Raczej konstruktor i destruktor. Reszta nie ma bezpośredniego związku ze sposobem przydzielania pamięci.
Rafal88K
Użytkownik
Użytkownik
Posty: 311
Rejestracja: 15 mar 2007, o 16:52
Płeć: Mężczyzna
Lokalizacja: Lublin
Podziękował: 28 razy
Pomógł: 54 razy

[C++] krótki kod do poprawienia

Post autor: Rafal88K »

amdfanatyk pisze:Raczej konstruktor i destruktor. Reszta nie ma bezpośredniego związku ze sposobem przydzielania pamięci.
Konstruktor kopiujący i operator przypisania mają związek z pamięcią, radzę o tym poczytać, a później dopiero pisać i się wymądrzać.

dynamiczna alokacja bez operator=

[ Dodano: 7 Kwietnia 2008, 18:26 ]
Może poniższy kod coś Ci wyjaśni, zwróć uwagę na konstruktor kopiujący i operator przypisania (generowane automatycznie) jak się zachowują, kompilator wykonuje płytkie kopiowanie

Kod: Zaznacz cały

#include <iostream>
using namespace std;

class k
{
    public:
        k(int inSize = 10);
        ~k();
        int get(int i);
        void set(int i, int val);
           
    private:
        int mSize;
        int* t;    
};

k::k(int inSize) : mSize(inSize)
{
    t = new int[inSize];
}

k::~k()
{
    delete[] t;
}

int k::get(int i)
{
    return t[i];
}

void k::set(int i, int val)
{
    t[i] = val;
}

int main(int argc, char** argv)
{
    int i;
    k a, c;
    k b = a;
    
    for(i = 0; i < 10; ++i) {
          a.set(i, i + 1);
    }
    
    c = a;
    c.set(0, 1000);
    
    cout << "Obiekt a: ";
    for(i = 0; i < 10; ++i) {
          cout << a.get(i) << " ";
    }
    cout << endl;
    
    cout << "Obiekt b: ";
    for(i = 0; i < 10; ++i) {
          cout << b.get(i) << " ";
    }
    cout << endl;
    
    cout << "Obiekt c: ";
    for(i = 0; i < 10; ++i) {
          cout << c.get(i) << " ";
    }
    
	cout << endl;
	system("pause");
	return 0;
}
Jak chcesz kogoś poprawiać to na początku upewnij się czy masz rację.
Awatar użytkownika
amdfanatyk
Użytkownik
Użytkownik
Posty: 98
Rejestracja: 27 mar 2005, o 14:59
Płeć: Mężczyzna
Lokalizacja: /dev/zero
Podziękował: 9 razy
Pomógł: 7 razy

[C++] krótki kod do poprawienia

Post autor: amdfanatyk »

Chodziło o to, co jest potrzebne, żeby zapewnić prawidłowe przydzielanie i zwalnianie pamięci a od tego jest konstruktor i destruktor. Twoje stwierdzenie "musisz" nie jest poprawne bo nic nie musi. Minimalny zestaw to konstruktor i destruktor.

Jeśli chcę wykorzystać swoją klasę do czegoś takiego:

Kod: Zaznacz cały

Klasa k = Klasa();
To potrzebuje konstruktora kopiującego.

Jeśli do czegoś takiego:

Kod: Zaznacz cały

Klasa k, l;
k = l;
To potrzebuje zdefiniować operator=.

Nie muszę używać swojej klasy ani w pierwszym ani w drugim kontekście więc nie muszę definiować konstruktora kopiującego i operatora=. Tak samo nie muszę definiować operatorów , == itd.
kawafis44
Użytkownik
Użytkownik
Posty: 474
Rejestracja: 22 paź 2007, o 20:55
Płeć: Mężczyzna
Lokalizacja: Gliwice
Podziękował: 416 razy
Pomógł: 2 razy

[C++] krótki kod do poprawienia

Post autor: kawafis44 »

czy tak będzie dobrze?

Kod: Zaznacz cały

#include <cstdlib>
#include <iostream>

using namespace std;

class DynA{
      private:
              int *tab;
              int size;
      public:
             DynA(); //create new array of 10 integers
             ~DynA(); //destructor; delete allocated memory
             int getValue (int index); //gets the value
             void setValue (int index, int newvalue); //sets the value specified by index
             void resize(int new_size);
}

int main(int argc, char *argv[])
{
    int DynA::getValue(int index)                { return DynA.index; }
    void DynA::setValue(int index, int newvalue) { DynA.index = newvalue }
    void DynA::resize(int new_size)              { DynA.size = new_size }

    DynA da;
    da.resize(100);
    da.setValue(5,10); //tab[5]=10;
    var = da.get(5); //var = tab[5];
    system("PAUSE");
    return EXIT_SUCCESS;
}
pzdr!
Rafal88K
Użytkownik
Użytkownik
Posty: 311
Rejestracja: 15 mar 2007, o 16:52
Płeć: Mężczyzna
Lokalizacja: Lublin
Podziękował: 28 razy
Pomógł: 54 razy

[C++] krótki kod do poprawienia

Post autor: Rafal88K »

amdfanatyk pisze:Klasa k = Klasa();
Tutaj na początku wywoływany jest konstruktor bezparametrowy, dopier późnej konstruktor kopiujący.

Kod: Zaznacz cały

    k a, c;
    k b = a; // konstruktor kopiujący
    c = a; // operator przypisania
amdfanatyk pisze:Twoje stwierdzenie "musisz" nie jest poprawne bo nic nie musi.
To "dobry" z Ciebie programista jeśli w swoich klasach nie zapewniasz prawidłowego przypisania i inicjalizacji obiektów innym obiektami.
amdfanatyk pisze:Nie muszę używać swojej klasy ani w pierwszym ani w drugim kontekście więc nie muszę definiować konstruktora kopiującego i operatora=.
Jeśli tego nie chcesz to podaj ich deklarację w private, żeby nie można było ich w ogóle wywoływać (nie podawaj definicji, konsolidator nie będzie jej szukał).
amdfanatyk pisze:Tak samo nie muszę definiować operatorów , == itd.
operator= i konstruktor kopiujący są automatycznie generowane przez kompilator i działają nie poprawnie w przypadku dynamicznej alokacji pamięci.

konstruktor kopiujący

Kod: Zaznacz cały

    k a;
    k b = a;
Jeśli zostanie wywołany destruktor obiektu a to w b będzie błędny wskaźnik.

operator=

Kod: Zaznacz cały

    k a, c;
    c = a;
Po przypisaniu (c = a) będziemy mieli wyciek pamięci bo to co na początku wskazywał wskaźnik w obiekcie c nie zostało skasowane, mało tego ten wskaźnik (z c) wskazuje na ten sam obszar pamięci co wskaźnik z obiektu a, więc jeśli coś zmieniamy to w obu obiektach.

Teraz zrozumiałeś? Dlaczego to takie ważne?

Skoro w Twoich kodach wycieki pamięci i błędne wskaźniki to normalka to nic nie musisz robić i wystarczy konstruktor kopiujący i operator= generowany przez kompilator.

[ Dodano: 7 Kwietnia 2008, 23:18 ]
kawafis44 pisze:czy tak będzie dobrze?
Tak będzie dobrze:

Kod: Zaznacz cały

#include <iostream>
using namespace std;

class DynA{
    public:
        DynA(int inSize = 10);
        DynA(const DynA& src);
        ~DynA();
        
        DynA& operator=(const DynA& rhs);
        
        void setValue(int i, int value);
        int getValue(int i) const;
        
        void resize(int newSize);

    private:
        void copy(const DynA& src);
        
        int* mTab;
        int mSize;
};

DynA::DynA(int inSize) : mSize(inSize)
{
    mTab = new int[inSize];
}

DynA::DynA(const DynA& src)
{
    copy(src);
}

DynA::~DynA()
{
    delete[] mTab;
}

DynA& DynA::operator=(const DynA& rhs)
{
    if(this == &rhs) {
        return *this;
    }
    
    delete[] mTab;
    
    copy(rhs);
    
    return *this;
}

void DynA::setValue(int i, int value)
{
     mTab[i] = value;
}

int DynA::getValue(int i) const
{
    return mTab[i];
}
       
void DynA::resize(int newSize)
{
     int* tmp = new int[newSize];
     
     for(int i = 0; i < mSize && i < newSize; ++i) {
         tmp[i] = mTab[i];
     }

     mSize = newSize;
     delete[] mTab;
     
     mTab = tmp;
}

void DynA::copy(const DynA& src)
{
     mSize = src.mSize;
     mTab = new int[mSize];
     
     for(int i = 0; i < mSize; ++i) {
         mTab[i] = src.mTab[i];
     }
}
    
int main(int argc, char** argv)
{
    const int kSize1 = 10;
    const int kSize2 = 25;
    DynA da(kSize1);
    DynA d;
    
    for(int i = 0; i < kSize1; ++i) {
        da.setValue(i, i + 1);
    }
    
    da.resize(25);
    
    for(int i = kSize1; i < kSize2; ++i) {
        da.setValue(i, 20);
    }
    
    d = da;
    d.setValue(0, 5);
    
    DynA c = d;
    c.setValue(0, 3);
    
    for(int i = 0; i < kSize2; ++i) {
        cout << da.getValue(i) << " ";
    }
    cout << endl;
    
    for(int i = 0; i < kSize2; ++i) {
        cout << d.getValue(i) << " ";
    }
    cout << endl;
    
    for(int i = 0; i < kSize2; ++i) {
        cout << c.getValue(i) << " ";
    }
    
    cin.get();
    return 0;
}
Awatar użytkownika
amdfanatyk
Użytkownik
Użytkownik
Posty: 98
Rejestracja: 27 mar 2005, o 14:59
Płeć: Mężczyzna
Lokalizacja: /dev/zero
Podziękował: 9 razy
Pomógł: 7 razy

[C++] krótki kod do poprawienia

Post autor: amdfanatyk »

Rafal88K pisze:
amdfanatyk pisze: To "dobry" z Ciebie programista jeśli w swoich klasach nie zapewniasz prawidłowego przypisania i inicjalizacji obiektów innym obiektami.
Reprezentujesz postawę osoby, która dopiero co ukończyła kurs C++ i teraz stara się wykorzystać cały zestaw ogromnych możliwości udostępnianych przez ten język. To czy jestem dobrym czy złym już zostało wiele razy sprawdzone a sama umiejętność pisania kodu jest tylko częścią tego, co musi umieć programista.

Do autora tematu:
Deklarację klasy zamykasz średnikiem bo jest to nowy typ danych, mimo tego, że jej deklarację zapisuje się zwykle "w pionie", użycie średnika w poniższym przypadku jest bardziej naturalne:

Kod: Zaznacz cały

enum TYP_DANYCH{CALKOWITY=0,POJEDYNCZA_PRECYZJA,PODWOJNA_PRECYZJA};
Metody muszą być zdefiniowane albo przy deklaracji klasy albo poza nią. Jeśli nie tworzysz plików nagłówkowych, to możesz robić to pierwsze, chociaż nie jest to wygodne w przypadku bardziej złożonych aplikacji.
Rafal88K
Użytkownik
Użytkownik
Posty: 311
Rejestracja: 15 mar 2007, o 16:52
Płeć: Mężczyzna
Lokalizacja: Lublin
Podziękował: 28 razy
Pomógł: 54 razy

[C++] krótki kod do poprawienia

Post autor: Rafal88K »

amdfanatyk pisze:Reprezentujesz postawę osoby, która dopiero co ukończyła kurs C++ i teraz stara się wykorzystać cały zestaw ogromnych możliwości udostępnianych przez ten język.
No nie wiem co to ma do tego co piszę, to że piszę jak powinno być prawidłowo? To znaczy, że dopiero co przeczytałem kurs?
amdfanatyk pisze:To czy jestem dobrym czy złym już zostało wiele razy sprawdzone a sama umiejętność pisania kodu jest tylko częścią tego, co musi umieć programista.
Wycieki pamięci i błędne wskaźniki?
amdfanatyk pisze:Metody muszą być zdefiniowane albo przy deklaracji klasy albo poza nią. Jeśli nie tworzysz plików nagłówkowych, to możesz robić to pierwsze, chociaż nie jest to wygodne w przypadku bardziej złożonych aplikacji.
Powód jest raczej taki, że jeśli razem z deklaracją podajesz definicję metod (w definicji klasy) to są one automatycznie inline.


amdfanatyk jak nie chcesz korzystać z konstruktora kopiującego i operatora przypisania to daj w private ich deklarację i to będzie poprawne.
Ostatnio zmieniony 26 maja 2008, o 01:38 przez Rafal88K, łącznie zmieniany 3 razy.
kawafis44
Użytkownik
Użytkownik
Posty: 474
Rejestracja: 22 paź 2007, o 20:55
Płeć: Mężczyzna
Lokalizacja: Gliwice
Podziękował: 416 razy
Pomógł: 2 razy

[C++] krótki kod do poprawienia

Post autor: kawafis44 »

nie rozumiem tych fragmentów kodu

Kod: Zaznacz cały

DynA& operator=(const DynA& rhs); 
oraz

Kod: Zaznacz cały

DynA::DynA(int inSize) : mSize(inSize)
{
    mTab = new int[inSize];
}
jak również

Kod: Zaznacz cały

DynA& DynA::operator=(const DynA& rhs)
{
    if(this == &rhs) {
        return *this;
    }
   
    delete[] mTab;
   
    copy(rhs);
   
    return *this;
} 
i co to jest &src

pozdrawiam!
Rafal88K
Użytkownik
Użytkownik
Posty: 311
Rejestracja: 15 mar 2007, o 16:52
Płeć: Mężczyzna
Lokalizacja: Lublin
Podziękował: 28 razy
Pomógł: 54 razy

[C++] krótki kod do poprawienia

Post autor: Rafal88K »

kawafis44 pisze:nie rozumiem tych fragmentów kodu

Kod: Zaznacz cały

DynA& operator=(const DynA& rhs);
To deklaracja operatora przypisania.
kawafis44 pisze:oraz

Kod: Zaznacz cały

DynA::DynA(int inSize) : mSize(inSize)
{
    mTab = new int[inSize];
}
Konstruktor zeroargumentowy (bezargumentowy lub domyślny, zależy jak kto to nayzwa ), nie podajesz wartości domyślnej bo one występują tylko w deklaracji, po dwukropku jest lista inicjalizacyjna (inicjalizuje pole w momencie jego tworzenia).
kawafis44 pisze:jak również

Kod: Zaznacz cały

DynA& DynA::operator=(const DynA& rhs)
{
    if(this == &rhs) {
        return *this;
    }
   
    delete[] mTab;
   
    copy(rhs);
   
    return *this;
}
Definicja operator= na początku musisz sprawdzić czy nie ma przypisania do samego siebie bo dalej jest kasowanie, następnie kopiujesz.
i co to jest &src
src to skrót od źródło, a znak ampersand oznacza referencję (alias zmiennej). Jeśli występuj jako parametr metody/funkcji to oznacza to przekazanie przez referencję, domyślnie w C++ parametry są przekazywane przez wartość - nie pracujesz na oryginale tylko na kopi, w przypadku obiektów kopiowanie jest nie wydajne, a tak przekazywany jest wskaźnik do obiektu, pracujesz na oryginale, jednak żeby uniemożliwić zmienienie wartości dodałem const - chociaż nie jest to do końca prawda bo jak byś chciał to mógłbyś zmienić ale nie będę Ci tym motał.
ODPOWIEDZ