-3

How to fix the error:

incompatible pointer to integer conversion assigning to 'int' from 'int *' dereference with *

Without changing the name of variables?

I want to use new and delete with this code:

#include <iostream>
using namespace std;

class rectangle
{
private:
    int *width,*length;
public:
    rectangle(int width,int length)
    {
        this->length=new int;
        this->width=new int;
        *width=width;
        *length=length;
    }

    void print()
    {
        cout<<*width * *length;
    }
};

int main()
{
    rectangle *r1;
    r1=new rectangle(5,6);
    r1->print();
    delete r1;
    return 0;
}
Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Can you explain what `*width=width;` is meant to do? – Scott Hunter Aug 03 '21 at 17:24
  • 1
    `*width=width;` -> `*(this->width)=width;` – UnholySheep Aug 03 '21 at 17:25
  • 4
    Your `rectangle` class doesn't need to store `int*` at all. Just eliminate those pointers. And use the constructor initialization section. – sweenish Aug 03 '21 at 17:25
  • You appear to be learning about `new` and `delete`. Those are very advanced topics in C++, unfortunately they tend to be taught early on even with a [good C++ book](https://stackoverflow.com/a/388282/4641116). Given smart pointers and containers, I don't think I've directly used a `new` or `delete` in my own code in the past 10 years. – Eljay Aug 03 '21 at 17:31
  • As sweenish answer suggests, I consider nonsense to allocate a single `int` (or any native type) with `new` (nor `malloc`, smart pointer, ...). – prapin Aug 03 '21 at 18:30

1 Answers1

1

The simplest fix is to initialize the variable instead of declare-then-assign.

#include <iostream>

using namespace std;

class rectangle
{
private:
 int *width,*length;
public:
 rectangle(int width,int length)
 {
     this->length=new int(length);  // Initialize with value
     this->width=new int(width);
    //  *width=width;
    //  *length=length;
 }
 void print()
 {
     cout<<*width * *length;
 }
};
int main()
{
 rectangle *r1;
 r1=new rectangle(5,6);
 r1->print();
 delete r1;
 return 0;
}

A big issue with your code as-is is that you violate the Rule of 5. Most notably in your case, you didn't create a destructor, and your rectangle class leaks memory. Deleting the rectangle pointer does not delete the memory that was allocated by the class itself.

Since you don't demonstrate any real reason to allocate those integers, it will be easier to just not. Then you don't have to worry about a destructor.

Some suggestions for better code are below.

#include <iostream>

// using namespace std;  // CHANGE: Bad practice

// CHANGE: Class names start with uppercase letter
class Rectangle {
 private:
  int width = 0;  // CHANGE: No pointers, default member initialization
  int length = 0;

 public:
  Rectangle(int width, int length) : width(width), length(length) {
    // CHANGE: Use ctor initialization section; body no longer needed
    //
    //  this->length=new int(length);  // Initialize with value
    //  this->width=new int(width);
    //  *width=width;
    //  *length=length;
  }
  // CHANGE: Replace print() with operator<<()
  friend std::ostream& operator<<(std::ostream& sout, const Rectangle& rect) {
    return sout << rect.width * rect.length;
  }
  //  void print()
  //  {
  //      cout<<*width * *length;
  //  }
};
int main() {
  Rectangle r1{5, 6};  // CHANGE: No demonstrated need to 'new' a rectangle;
                       // initialize > declare/assign
  std::cout << r1 << '\n';  // CHANGE: Can now std::cout directly
  // delete r1;
  return 0;
}

Finally, here is the code above, with all comments removed. I know sometimes this helps with read-ability when there's a mix of old/new code. It also, to me, highlights how much cleaner some of the new practices can be when you can format without worrying about keeping dead code.

#include <iostream>

class Rectangle {
 private:
  int width = 0;
  int length = 0;

 public:
  Rectangle(int width, int length) : width(width), length(length) {}

  friend std::ostream& operator<<(std::ostream& sout, const Rectangle& rect) {
    return sout << rect.width * rect.length;
  }
};

int main() {
  Rectangle r1{5, 6};
  std::cout << r1 << '\n';

  return 0;
}
sweenish
  • 4,793
  • 3
  • 12
  • 23
  • Addendum: In C++ `Rectangle r1(5, 6);` is almost always preferable to `Rectangle* r1; r1 = new Rectangle(5, 6);` and then later having to `delete r1;` all in the same function. Occasionally you're forced to use `new`, but you shouldn't use `new` until after Automatic allocation and a [RAII-wrapped](https://stackoverflow.com/questions/2321511/what-is-meant-by-resource-acquisition-is-initialization-raii) container like a Library Container or Smart Pointer have both been investigated and rejected. It is likely only used in this answer by Asker request. – user4581301 Aug 03 '21 at 18:22
  • True. I left the `new`/`delete` in `main()` because I assumed they're practicing with it. I'm adding an edit about why the pointers in the class was a bad idea and the fact that they leaked memory. I'll probably also add in the uniform initialization while I'm at it. – sweenish Aug 03 '21 at 18:29
  • EDIT notes: More for OP than someone who would look at edit history. Since there was no demonstrable need for allocation, I removed it entirely from the second example. – sweenish Aug 03 '21 at 18:35