Błąd przy char* :: 4programmers.net

Rejestracja:prawie 13 lat
Ostatnio:ponad 12 lat
Postów:21

Witam,

Oto polecenie do programu : class String {char texe[256]}; napisać operator+=, operator[], operator=. Ja dorobiłem do tego konstruktor w celu łatwego sprawdzenia ale dodaje mi spacje i jakiego krzaczka po słowie "elo". Nie wiem czy mi mózg przestał już pracować czy co ale nie bardzo mogę zauważyć błąd. Z góry dziękuję :)

Kopiuj

#include <cstdlib>
#include <iostream>

using namespace std;

class String
{
      public:
      char text[256];
      unsigned pos;
      //public:
             String(char *word)
             {
                         int i = 0;
                         while (word[i] != '\0')
                         {
                               text[i] = word[i];
                               i++;
                         }
             }
             char operator[](unsigned n)
             {
                  return text[n];
             }
             void operator+=(String s)
             {
                  for (int i = pos; i < 256; i++)
                      text[i] = s[i - pos];
             }
             void operator=(String s)
             {
                  for (int i = 0; i < 256; i++)
                      text[i] = s[i];
             }
             friend ostream& operator<<(ostream& out, String word)
             {
                    int i = 0;
                    while (word[i] != '\0')
                    {
                          out << word[i];
                          i++;
                    }
                    return out;
             }
};
             
int main()
{
    String word("elo");
    String word2("bieda");
    cout << word << word2 << endl;
    
    
    system("PAUSE");
    return 0;

Krycho

2012-08-11 22:20

Krycho2012-08-11 22:20

Rejestracja:prawie 14 lat
Ostatnio:prawie 3 lata
Postów:353

Kopiuj

 while (word[i] != '\0')
 {
      text[i] = word[i];
      i++;
 }

Zagadka. Co znajduje sie na koncu podanego wyrazu(konkretnie za)? Dokad wypisuje sie taka tablice charow?

edytowany 1x, ostatnio: Krycho 2012-08-11 22:21

Lena(R)

2012-08-12 00:22

Lena(R)2012-08-12 00:22

Rejestracja:ponad 14 lat
Ostatnio:ponad 4 lata
Postów:98

Kopiuj

            
String(char *word)
{
      int i = 0;
      while (word[i] != '\0')
      {
           text[i] = word[i];
           i++;
      }
}

Ten kod, konstruktor Twojej klasy ma dość znaczące błędy i w nim jest problem. Jedna linijka naprawia problem, ale najpierw co jest źle:

Nie masz pewności że argument 'word' będzie miał znak '\0'.
Nawet jeśli będzie miał, to nie ma pewności, że występuje on na pozycji mniejszej niż rozmiar tablicy text (która może pomieścić 256 znaków).
Najważniejszy błąd (i dlatego że to tablica statyczna, dość mało rażący), nigdzie nie przepisujesz, ani nie przypisujesz znaku NULL czy '\0' (jak kto woli) do własnej tablicy. Przez to, kiedy wypisujesz dane, ostream nigdy nie zatrzymuje się po słowie podanym w konstruktorze, tylko wypisuje wszystko do końca rozmiaru tablicy.
Dwie magiczne poprawki:

Kopiuj

            
String(char *word)
{
      int i = 0;
      while (word[i] != '\0' && i < 255)    // to nie przypadek, że i < 255, a nie i < 256, ponieważ jeśli do pozycji 254 nie znajdzie NULL, to na ostatniej pozycji trzeba go recznie wpisać. Jeśli przy i == 255 wejdzie do pętli, poza nią będzie i == 256, co wykracza poza zakres tablicy i błędnie przypisze NULL poza pętlą
      {
           text[i] = word[i];
           i++;
      }
      text[i] = '\0';
}

Poza tym, jeszcze dwie uwagi techniczne (być może na razie specjalnie zostawione). W operator[] dobrze by było sprawdzać, czy argument nie wykracza poza zakres tablicy, zawsze ktoś może podać 10000. Niech wtedy zwraca NULL i z głowy. Druga, to chyba zostawione przez nieuwagę: obstawiam, że pole klasy 'pos' ma wskazywać znak NULL tablicy 'text' żeby było wiadomo gdzie dopisywać kolejne znaki w operator+=. Dobry pomysł, tylko że ta zmienna nigdzie nie jest zainicjalizowana (powinna mieć wartość 0 z racji pola klasy, ale u mnie sypie błędem i wpisana jest tam przypadkowa wartość) i nigdy nie zmieniasz jej wartości potem. Także obecnie operator+= i operator= robią dokładnie to samo (jeśli przyjmiemy że pos == 0). Należałoby 'pos' ustawić na

Kopiuj

pos = i;

jako ostatnia linijka w konstruktorze, no i potem odpowiednio zmieniać jej wartości w pętli for w operator+=. Ja to widzę np. tak:

Kopiuj

for(int i = 0; i < 256 && s[i] != NULL && pos < 255; i++, pos++)   // ponownie trick z <255, a nie <256, żeby było miejsce na NULL
{
     text[pos] = s[i];
}
text[pos] = NULL;

Tak bym to wszystko widział. Poza tym, operatory += i = powinny zwracać wartości lub referencje do swoich klas, a nie void mimo że teraz to jakoś działa.

krwq 2012-08-18 07:04

nie latwiej do while uzyc zamiast while + linijka na koncu?

kidziman

2012-08-13 08:30

kidziman2012-08-13 08:30

Rejestracja:prawie 13 lat
Ostatnio:ponad 12 lat
Postów:21

Dzięki wielkie za odp, faktycznie wiele pomogło. Mam tylko pytanie, dlaczego te operatory (+=, =) powinny zwracać wartości lub referencje do swoich klas? No bo ja rozumiem to tak, że używając na przykład operatora = na obiektach s1 i s2 (s1 = s2) do s1 przepisuje s2. dlaczego więc powinienem coś zwracać? A i mam jeszcze jedno pytanie, mam w klasie zaimplementowany operator++ dla przykładu, jak mogę go użyć jeszcze w deklaracji tej klasy (np. class hour { operator ++ {...} i teraz jak użyć dla tego hour operatora ++ }). Wielkie dzięki za odp :)

Macron 2012-08-13 09:10

Zwracać powinieneś, aby móc tworzyć konstrukcje typu a = b = c =d;

Krycho 2012-08-13 11:21

lub cout << (a+b) gdzie sa to obiekty twojej klasy zwracajac void bedzie fail

Macron 2012-08-13 11:26

@up nie + tylko +=! + przecież: 1. nie jest w tej klasie 2. to raczej by było wiadomo.

Krycho 2012-08-13 11:42

Kompletnie ci nie zrozumialem.

kidziman

2012-08-17 19:18

kidziman2012-08-17 19:18

Rejestracja:prawie 13 lat
Ostatnio:ponad 12 lat
Postów:21

Dzięki za odpowiedzi, wyjaśniło mi się skąd to zwracanie. Mam jeszcze pytanie do innego problemu z którym się natknąłem, a że pewnie problem jest gdzieś podobny to zamieszczę w tym temacie żeby nie robić śmietnika. Otóż mam za zadanie zrobić szablon klasy List przechowywującej elementy typu T na liście dwukierunkowej. Oto kod:

Kopiuj

 
#include <cstdlib>
#include <iostream>

using namespace std;

template <class T>
class List
{
      struct list
      {
             list* prev;
             list* next;
             T val;
      };

      list* tab;
      
      public:
             List() {tab = NULL;}
             ~List()
             {
                    list* pom;
                    while (tab->prev != NULL)
                    {
                          pom = tab;
                          tab = tab->prev;
                          delete pom;
                    }
             }
             list* get()
             {
                   return tab;
             }
             void pushback(const T& t);
             void deletefront();
             List(List& temp);
             friend ostream& operator<<(ostream& out, List<T>& l)
             {
                    list* temp = new list;
                    temp = l.get();
                    while (temp->prev != NULL)
                          temp = temp->prev;
                    if (temp->next != NULL)
                    {

                                   while (temp->next != NULL)
                                   {
                                         out << (*temp).val << endl;
                                         temp = temp->next;
                                   }
                                   out << (*temp).val << endl;
                    }
                    return out;
             }
};

template <class T>
void List<T>::pushback(const T& t)
{
     if (tab == NULL)
     {
             tab = new list;
             tab->prev = NULL;
             tab->next = NULL;
             tab->val = t;
     }
     else
     {
         list* temp;
         temp = new list;
         temp->val = t;
         temp->prev = tab;
         tab->next = temp;
         tab = temp;
     }
}

template <class T>
void List<T>::deletefront()
{
     list* temp;
     temp = tab;
     while (tab.prev != NULL)
           tab = tab->prev;
     if (tab->next != NULL)
        tab = NULL;
     else
     {
         list* temp2;
         temp2 = new list;
         temp2 = tab;
         tab = tab->next;
         tab->prev = NULL;
         delete temp2;
     }
     tab = temp;
}

template <class T>
List<T>::List(List& temp)
{
     list* t;
     t = new list;
     t = temp.get();
     while (t->prev != NULL)
           t = t->prev;
     if (t->next != NULL)
     {
     tab = new list;
     tab = t;
     while (t->next != NULL)
     {
           t = t->next;
           tab->next = new list;
           tab->next = t;
           tab = tab->next;
           tab->prev = t->prev;
     }
     }
}
                          

int main()
{
    List<int> l;
    List<int> temp;
    
    l.pushback(5);
    l.pushback(6);
    l.pushback(8);
    cout << l;
    
    system("PAUSE");
    return 0;
}

Dodam na wstępie że mechanizm z jednym wskaźnikiem i w każdej funkcji wracanie do początku listy to wymóg (dlatego nie ma *begin i *end na przykład tylko jest jeden *tab). W tej części wykorzystuję tylko pushback i operator<< więc pewnie gdzieś tam jest błąd (przypuszczam że w pushback) ale nie jestem już w stanie do tego dojść ;/ Dzieki za wszystkie odpowiedzi

Lena(R)

2012-08-18 04:02

Lena(R)2012-08-18 04:02

Rejestracja:ponad 14 lat
Ostatnio:ponad 4 lata
Postów:98

Ponieważ Twój kod z jakichś przyczyn się u mnie nie chce skompilować, będę trochę teoretyzował skąd mogą się brać problemy. Może coś to pomoże.

Skoro zawsze wracasz na początek listy, to czemu w funkcji pushback dodajesz nowy element za tym na co obecnie wskazuje wskaźnik tab? Nie masz pewności przecież, że jest on na końcu (a przynajmniej ja nie widzę nigdzie tego w kodzie). Poza tym, kiedy dodajesz nowy element to nie wpisujesz jawnie, że

Kopiuj

temp->next = NULL

Nie jest pewien czy na pewno ten wskaźnik sam się tak domyślnie ustawia, a w pozostałych funkcjach szukasz momentu kiedy któryś 'next' będzie równy NULL. Przez to może nigdy go nie znaleźć i tutaj jest potencjalny problem.

To raczej nie jest problemem, związanym z błędnym działaniem programu, ale w tym miejscu dochodzi do wycieków pamięci. W kilku funkcjach piszesz coś takiego (tutaj przytoczę fragment kodu operator<<):

Kopiuj

list* temp = new list;
temp = l.get();

W paru miejscach robisz coś takiego, a to co robisz, to tworzysz dynamicznie obiekt, po czym za chwile nadpisujesz wskaźnik na niego tym co zwróci Ci funkcja get(). Czyli gdzieś w pamięci wisi nowy obiekt którego nigdy nie będziesz miał możliwości ani usunięcia, ani wykorzystania. Powinieneś zrobić tak:

Kopiuj

list* temp = l.get();

W tym miejscu powinieneś też sprawdzać czy get() nie zwraca NULL, bo Twoja implementacja to dopuszcza (jeśli nie dasz żadnego pushback(), to ta funkcja zwróci NULL).

Funkcja deletefront() jest źle napisana. Ma parę błędów. Przede wszystkim, to chyba przez roztargnienie instrukcja if-else ma błędny warunek, bo w tej formie nie ma sensu:

Kopiuj

    if (tab->next != NULL)   // jesli tab ma jakieś next to tab = NULL i...koniec?
        tab = NULL;
     else            // a jak tab->next == NULL to probujemy cos usunac
     {
         list* temp2;
         temp2 = new list;
         temp2 = tab;
         tab = tab->next;   // przesunac tab na jakieś jego tab->next, czyli na NULL ?
         tab->prev = NULL; // a potem tab->prev... ups, przeciez juz tab == NULL, więc jak można do tab->prev coś przypisać?
         delete temp2;
     }

Pewnie miało być if( tab-> next == NULL) i reszta wtedy ma sens i jest prawie dobrze. Prawie, bo:

Kopiuj

    if (tab->next == NULL)
        tab = NULL;        // a co z tym? jesli tab->next == NULL nie znaczy, ze tab nie wskazuje na jakiś dynamicznie stworzony obiekt, więc nie można tak
     else
     {
         list* temp2;
         temp2 = new list;
         temp2 = tab;
         tab = tab->next;
         tab->prev = NULL;
         delete temp2;
     }

Przed tab = NULL, dodaj delete tab i będzie raczej dobrze (ale jak wspomniałem na wstępie, teoretyzuje, kompiluje w oczach, bo VS nie chce skompilować Twojego kodu, bo czegoś nie widzi, ale to wina po mojej, nie po Twojej stronie).

Tyle zauważyłem z teoretycznych rozważań. Może coś pomogłem, a może tylko straciłem czas. Sam ocenisz. W razie pytań pisz, postaram się pomóc.

kidziman

2012-08-18 15:19

kidziman2012-08-18 15:19

Rejestracja:prawie 13 lat
Ostatnio:ponad 12 lat
Postów:21

Super, ta odpowiedź całkowicie mi pomaga i rozwiązuje moje wątpliwości :) Dzięki wielkie, co do punktu dwa to sam nie wiem, chyba już zmieniałem co się da żeby sprawdzić czy działa, 3 oczywiście niedopatrzenie. Teraz wszystko działa ok :) Dzięki jeszcze raz.