the c destructor is not working properly-CodePudding

The problem in this code is that the second object cb2 is not created nor dispalyed. There's an infinite loop after displaying the first object cb1, so I think the problem is with the destrutor at the end of the code. But, I can't figure it out.

#include <iostream>
#include <string>
#include <windows.h>

#ifndef SETCOLORFUN_H_INCLUDED
#define SETCOLORFUN_H_INCLUDED
#endif

using namespace std;

class ColoredBox {

private:
  int *length;
  int *width;
  char character;
  int color;
  int maxarea;

public:
  void setColor(int ForgC) {
    WORD wColor;

    HANDLE hStdOut = GetStdHandle(STD_OUTPUT_HANDLE);
    CONSOLE_SCREEN_BUFFER_INFO csbi;

    // We use csbi for the wAttributes word.
    if (GetConsoleScreenBufferInfo(hStdOut, &csbi)) {
      // Mask out all but the background attribute, and add in the forgournd
      // color
      wColor = (csbi.wAttributes & 0xF0)   (ForgC & 0x0F);
      SetConsoleTextAttribute(hStdOut, wColor);
    }
    return;
  }

  ColoredBox(int boxlen, int boxwid, char charac = '#', int colo = 15) {
    *length = boxlen;
    *width = boxwid;
    character = charac;
    color = colo;
    maxarea = 0;
  }

  int getArea() { return (*length) * (*width); }

  int getMaxArea() {
    if (maxarea < ((*length) * (*width))) {
      maxarea = ((*length) * (*width));
    }
    return maxarea;
  }

  void display() {
    setColor(color);
    for (int i = 0; i < *length; i  ) {
      for (int j = 0; j < *width; j  ) {
        cout << character;
      }
      cout << endl;
    }
    setColor(15);
  }

  void displayTransposed() {
    setColor(color);
    for (int i = 0; i < *width; i  ) {
      for (int j = 0; j < *length; j  ) {
        cout << character;
      }
      cout << endl;
    }
    setColor(15);
  }

  void displayWider() {
    setColor(color);
    for (int i = 0; i < *length; i  ) {
      for (int j = 0; j < *width; j  ) {
        cout << character << " ";
      }
      cout << endl;
    }
    setColor(15);
  }

  void displayChess(int color2) {

    for (int i = 0; i < *length; i  ) {
      for (int j = 0; j < *width; j  ) {
        if (j % 2 == 0 && i % 2 != 0) {
          setColor(color2);
        }
        if (i % 2 == 0 && j % 2 != 0) {
          setColor(color2);
        }
        if (j % 2 == 0 && i % 2 == 0) {
          setColor(color);
        }
        if (j % 2 != 0 && i % 2 != 0) {
          setColor(color);
        }
        cout << character;
      }
      cout << endl;
    }
    setColor(15);
  }

  ~ColoredBox() {
    delete[] length;
    delete[] width;
  }
};

int main() {
  int len, wid, col, col2, max;
  char boxChar;
  cout << "Enter length and width of the box separated by a space: ";
  cin >> len >> wid;
  ColoredBox *cb1;
  cb1 = new ColoredBox(len, wid);
  cb1->display();
  cout << "Set the box to another color: ";
  cin >> col;
  cout << "Displaying Transposed: " << endl;
  cb1->setColor(col);
  cb1->displayTransposed();
  cout << "Displaying Wider: " << endl;
  cb1->displayWider();
  cout << "Displaying Chess, enter the other color: " << endl;
  cin >> col2;
  cb1->displayChess(col2);
  cout << endl << "Area=" << cb1->getArea();
  cout << endl << "Max Area=" << cb1->getMaxArea();
  delete cb1;
  cout << "Enter length,width,color,character of the box separated by spaces: ";
  cin >> len >> wid >> col >> boxChar;
  ColoredBox *cb2;
  cb2 = new ColoredBox(len, wid, col, boxChar);
  cb2->display();
  cout << "Displaying Transposed: " << endl;
  cb2->displayTransposed();
  cout << "Displaying Wider: " << endl;
  cb2->displayWider();
  cout << "Displaying Chess, enter the other color: " << endl;
  cin >> col2;
  cb2->displayChess(col2);
  cout << "Displaying original agaigetArean:" << endl;
  cb2->display();
  cout << endl << "Area=" << cb2->getArea();
  cout << endl << "Max Area=" << cb2->getMaxArea();
  delete cb2;
  return 0;
}

You can run this code the first object cb1 is created and is displayed properly in the three display functions, also i deleted cb1 and tried using cb2 only and it worked so i see that the problem is the destructor

CodePudding user response：

This is undefined behavior:

  ColoredBox(int boxlen, int boxwid, char charac = '#', int colo = 15) {
    *length = boxlen;
    *width = boxwid;

length and width are uninitialized values (one UB) so those contains some garbage values and then those pointers are dereferenced (second UB) so you write values boxlen boxwid to some random location of memory.

At the end in a destructor you are try to free memory which is not allocated, since those pointers are pointing to some random place.

Why you use pointers there at all? Why you try to use heap (I do not see reason to do it)?

CodePudding user response：

As it seems you are new to C and you don't have the basic understanding of pointers

From MSDN

A pointer is a variable that stores the memory address of an object

The reason to use pointers is when you want to have something store at the heap, so you can access it when it goes out of scope. Check this stack overflow answer for more information

In this particular example it seems there is no need to use pointers. So change your pointer variables to integers. Remove the dereferences and the deletes[] from the destructor. Of course your code has some other problems, but here is the code without using pointers.

#include <iostream>
#include <string>
#include <windows.h>

#ifndef SETCOLORFUN_H_INCLUDED
#define SETCOLORFUN_H_INCLUDED
#endif

using namespace std;

class ColoredBox {

private:
    int length;
    int width;
    char character;
    int color;
    int maxarea;

public:
    void setColor(int ForgC) {
        WORD wColor;

        HANDLE hStdOut = GetStdHandle(STD_OUTPUT_HANDLE);
        CONSOLE_SCREEN_BUFFER_INFO csbi;

        // We use csbi for the wAttributes word.
        if (GetConsoleScreenBufferInfo(hStdOut, &csbi)) {
            // Mask out all but the background attribute, and add in the forgournd
            // color
            wColor = (csbi.wAttributes & 0xF0)   (ForgC & 0x0F);
            SetConsoleTextAttribute(hStdOut, wColor);
        }
        return;
    }

    ColoredBox(int boxlen, int boxwid, char charac = '#', int colo = 15) {
        length = boxlen;
        width = boxwid;
        character = charac;
        color = colo;
        maxarea = 0;
    }

    int getArea() { return (length) * (width); }

    int getMaxArea() {
        if (maxarea < ((length) * (width))) {
            maxarea = ((length) * (width));
        }
        return maxarea;
    }

    void display() {
        setColor(color);
        for (int i = 0; i < length; i  ) {
            for (int j = 0; j < width; j  ) {
                cout << character;
            }
            cout << endl;
        }
        setColor(15);
    }

    void displayTransposed() {
        setColor(color);
        for (int i = 0; i < width; i  ) {
            for (int j = 0; j < length; j  ) {
                cout << character;
            }
            cout << endl;
        }
        setColor(15);
    }

    void displayWider() {
        setColor(color);
        for (int i = 0; i < length; i  ) {
            for (int j = 0; j < width; j  ) {
                cout << character << " ";
            }
            cout << endl;
        }
        setColor(15);
    }

    void displayChess(int color2) {

        for (int i = 0; i < length; i  ) {
            for (int j = 0; j < width; j  ) {
                if (j % 2 == 0 && i % 2 != 0) {
                    setColor(color2);
                }
                if (i % 2 == 0 && j % 2 != 0) {
                    setColor(color2);
                }
                if (j % 2 == 0 && i % 2 == 0) {
                    setColor(color);
                }
                if (j % 2 != 0 && i % 2 != 0) {
                    setColor(color);
                }
                cout << character;
            }
            cout << endl;
        }
        setColor(15);
    }

    ~ColoredBox() {}
};

int main() {
    int len, wid, col, col2, max;
    char boxChar;
    cout << "Enter length and width of the box separated by a space: ";
    cin >> len >> wid;
    ColoredBox* cb1;
    cb1 = new ColoredBox(len, wid);
    cb1->display();
    cout << "Set the box to another color: ";
    cin >> col;
    cout << "Displaying Transposed: " << endl;
    cb1->setColor(col);
    cb1->displayTransposed();
    cout << "Displaying Wider: " << endl;
    cb1->displayWider();
    cout << "Displaying Chess, enter the other color: " << endl;
    cin >> col2;
    cb1->displayChess(col2);
    cout << endl << "Area=" << cb1->getArea();
    cout << endl << "Max Area=" << cb1->getMaxArea();
    delete cb1;
    cout << "Enter length,width,color,character of the box separated by spaces: ";
    cin >> len >> wid >> col >> boxChar;
    ColoredBox* cb2;
    cb2 = new ColoredBox(len, wid, col, boxChar);
    cb2->display();
    cout << "Displaying Transposed: " << endl;
    cb2->displayTransposed();
    cout << "Displaying Wider: " << endl;
    cb2->displayWider();
    cout << "Displaying Chess, enter the other color: " << endl;
    cin >> col2;
    cb2->displayChess(col2);
    cout << "Displaying original agaigetArean:" << endl;
    cb2->display();
    cout << endl << "Area=" << cb2->getArea();
    cout << endl << "Max Area=" << cb2->getMaxArea();
    delete cb2;
    return 0;
}

I suggest you do some research and C pointers and memory management.

Good luck!