-1

I faced an error as shown below in this->p=b. there was no error if I type this->p=new int(b);. please enlighten me.

  class A{
      int *p;
  public:
      A():p(new int){}
      ~A(){delete p;}
      A(const A&) = delete;
      A& operator=(const A&) = delete;
      void GetValue(int b);
  };

  void A::GetValue(int b){
      this->p=new int;
      this->p=b; // error int can not be assigned to an entity of type int
  }
user10293779
  • 103
  • 6
  • 2
    You can assign `int*` to `int*`, but you can't assign `int` to `int*`. – Evg Sep 14 '18 at 09:23
  • You *could* assign `this->p = &b;`, but you wouldn't, as once the member fn is done, `p` is left holding a dangling pointer to nowhere. You are, however, left with a stellar example of "just because it compiles doesn't mean it's right" to put on the mantle.(no more right than the memory leaks present already). – WhozCraig Sep 14 '18 at 09:26
  • Wouldn't it be worth to change `int *p` to `int p`? So, the storage for `p` is managed by its parent `class A`. – Scheff's Cat Sep 14 '18 at 09:31
  • 1
    @Scheff I think it is difficult to say what is best, as I doubt this will be the full prupose of the code. The OP should explain a little bit what his intentions are?!?! –  Sep 14 '18 at 09:38

2 Answers2

1

You cannot asign an integer to an integer pointer. You have to dereference it first

*(this->p)=b;

Or shorter:

this->p=new int(b);

But to be honest I do not understand why this method would be called GetValue, as it sets a value. If you explain what the purpose of the method is, maybe we can explain more. It is likely your program will not do what you desire, even though it may compile.

there was no error if I type "this->p=new int(b);"

This is constructing an int with value b which is legal code

Further possible design issues:

 void A::GetValue(int b){
     this->p=new int;
     this->p=b; // error int can not be assigned to an entity of type int
  }

When you do this->p=new int; you have a possible memory leak, i.e. when the pointer was already pointing to some memory. You will have to check that first before allocating new memory. The usual approach would be to introduce smart pointers, i.e. std::unique_ptr.

So a valid implementation could look like that

class A{
    int *p;
public:
    A():p(nullptr){}
    ~A(){delete p;}
    A(const A&) = delete;
    A& operator=(const A&) = delete;
    void GetValue(int b);
};

void A::GetValue(int b){

    delete p;

    this->p=new int(b);
}

Or better

class A{
    std::unique_ptr<int> p;
public:
    A(const A&) = delete;
    A& operator=(const A&) = delete;
    void GetValue(int b);
};

void A::GetValue(int b){

    p.reset(new int(b));
}
  • @Scheff Agree. I also mentioned unique_ptr as this would be the better practice in any way –  Sep 14 '18 at 09:37
  • 1
    The best practice in this case is not to use pointers *at all*. Have the member simply be `int` and store by value. And the "possible memory leak" is a *guaranteed* memory leak if `GetValue` is ever called. – WhozCraig Sep 14 '18 at 09:38
  • @WhozCraig I agree again, but the OP has pointers in the design and I doubt that this is the whole program. I just assume there is a reason to use pointers. Otherwise we will have to consider the question as not answerable –  Sep 14 '18 at 09:40
  • @Pi I see this question as follow-up to her/his last: [SO: Why is destructor hanging](https://stackoverflow.com/q/52306425/7478597). So, this looks like she/he is trying to master the "raw pointer hell". But, I agree with WhozCraig - at best you avoid it in the sense of [Kate Gregory's Stop teaching C](https://www.youtube.com/watch?v=YnWhqhNdYyk). – Scheff's Cat Sep 14 '18 at 10:10
  • 1
    @Scheff I cannot (and do not want to) make a case against that ;) The OP has two answers available which cover best practices AND how to solve the pointer problem, so it should be fine either way. –  Sep 14 '18 at 10:24
  • thank all you for the valuable inputs. I am learning raw pointer now, trying to use a class with pointers to store then return multiple arrays. Along the practices, i encountered errors as shown. I feel better with your explanations and illustrations. After completed with pointer, next, i will learn to use 'vector' and 'smart pointer', or 'reference' as suggested. Thank you again – user10293779 Sep 14 '18 at 15:21
  • this->p refers to a memory address. *(this->p) re-reference pointer to accept a value. thank you for explanation – user10293779 Sep 15 '18 at 15:50
  • @user10293779 The `*` dereferences a pointer, which allows you to manipulate the underlaying value. So yes, I think you understood correctly. –  Sep 15 '18 at 22:29
1

To grant proper storage for what a pointer points to, is one of the most central problems in C++ code. Hence, a lot of effort has been done to eliminate the usage of raw pointers/new/delete as much as possible.

I struggled a bit to construct a simple example where a raw pointer really makes sense. So, assuming that in the following case, the raw pointer has been intentionally chosen to prevent unnecessary copying of constant data.

This constructed sample differs from OP's that in my case, struct A is not responsible for life-time management of pointee. Instead, the pointer is used to refer to something which is life-time-managed outside of struct A:

struct A {
  const char *const name;

  explicit A(const char *const name): name(name) { }
  ~A() = default;
  A(const A&) = delete;
  A& operator=(const A&) = delete;
  const char* getName() const { return name; }
};

could be used e.g. this way:

A a("a");

So, struct A is not responsible for life-time management of the passed pointer – no new or delete inside of struct A.

However, this is also a weakness of the concept demonstrated in this counter example:

std::string name;
name = "a";
A a(name.c_str());
// Now, disaster begins
name = "bcdefghijklmnopqrstuvwxyz";
// as the previous storage in std::string name may be released
// but a is still alive.

struct A trusts in that "outside" grants proper life-time of pointee. That could be over-enthusiastic as shown above. (My practical experience: Even well documented assumptions may be ignored by other programmers. Even with knowing the doc., this might be violated accidentally because of losing overview in too much code.)

Live Demo on coliru

Note: The output on coliru was:

a.getName(): ''

I expected a crash but, actually, this is Undefined Behavior – the former is as probable as the latter (as anything else, including "seems to work").

A possible fix:

struct A {
  const std::string name;

  explicit A(const char *const name): name(name) { }
  ~A() = default;
  A(const A&) = delete;
  A& operator=(const A&) = delete;
  const char* getName() const { return name.c_str(); }
};

Now, struct A makes a private copy of passed string in pointer where it manages itself the life-time of. This comes with the cost of some extra memory but the advantages are the lowered danger of wrong usage.

Instead of fiddling with new char[] I just used std::string, and all life-time issues are managed properly "under the hood".

To prevent raw pointers also, it could be modified to:

struct A {
  const std::string name;

  explicit A(const std::string &name): name(name) { }
  ~A() = default;
  A(const A&) = delete;
  A& operator=(const A&) = delete;
  const std::string& getName() const { return name; }
};

which still can be used with:

A a("a"); // making an implicit conversion of (char[2])"a" to std::string
Scheff's Cat
  • 19,528
  • 6
  • 28
  • 56
  • Nice explaination, but it fails to answer the OPs question a bit, which is about why can I do one thing, but not another. It is not about how to design the class. –  Sep 14 '18 at 09:47
  • @Pi I assume the example is educational to learn how pointer works. This is C++. At best, you don't need raw pointers. I'm not sure how to elaborate this at shortest as well as at best... – Scheff's Cat Sep 14 '18 at 09:49