If you declare a local pointer with the same name & type as a pointer data member ...

I'm working on Chapter 8, Exercise 6 from Ivor Horton's Beginning Visual C++ 2008.

This example uses code written in a previous exercise and builds on it. These are the directions:

Quote:

---

Modify the stack example from Exercise 7 in the previous chapter so that the size of the stack is specified in the constructor and dynamically allocated.

---

Here's the original header file contents for Chapter 7, Exercise 7:

Code:

---

class CStack
{
public:
  CStack() : next(0) {}
  void push(int i);
  int pop();
  void print();
private:
  int list[100];
  int next;
};

---

Here's my current modified copy of it with all comments removed (also here):

Code:

---

class CStack
{
public:

    CStack(int requested_size = 1);
    CStack(const CStack& s);
    ~CStack();

    CStack& operator=(const CStack& rhs);

    void push(int value);
    int pop();
    void print();

private:
    int* plist;
    int next;
    int stack_size;
};

---

After comparing my code against the author's solution, I realized I went a bit overboard and ended up with something else other than what the author intended for the reader to create.

Inside of the push() member function there is a check for whether the max capacity for the stack has been reached. If not, the next available slot (as I've taken to calling it) is used to store a value.

If there is no more room, the stack is "grown" by:

1. Creating a temporary dynamic array
2. Copying the existing array to the temporary
3. Copying new value (argument to pop() function) to temporary
4. Deleting the existing array
5. Creating a new one sized to the temporary array
6. Copying everything back over
7. Deleting the temporary

I'm sure that's enough to make anyone of skill cringe, but I'm new, so I'm not beating myself up too much.

While working on this exercise, one mistake I know I made was declare a pointer within pop() with the same name as a data member pointer. I didn't realize I had done it until g++ gave me a warning that the locally declared variable wasn't being used.

Once I realized what I did, I changed all usages of plist to this->plist to make it stand out more (to me anyway).

Code:

---

-            int* plist = new int[new_stack_size];
+            this->plist = new int[new_stack_size];

---

So, all of that said, did my declaration of plist within pop() mask or hide this->plist?

• Current project code (r17) here
• cstack.cpp, r17

P.S.

If I wasn't clear, I'm referring to when this line:

Code:

---

this->plist = new int[new_stack_size];

---

was

Code:

---

int* plist = new int[new_stack_size];

---

when I got the warning.

Thanks in advance for any help!

Edit:

The original code (if anyone is interested) can be found here:

http://www.wrox.com/WileyCDA/WroxTit...-DOWNLOAD.html

Quote:

---

Originally Posted by deoren

1. Creating a temporary dynamic array
2. Copying the existing array to the temporary
3. Copying new value (argument to pop() function) to temporary
4. Deleting the existing array
5. Creating a new one sized to the temporary array
6. Copying everything back over
7. Deleting the temporary

I'm sure that's enough to make anyone of skill cringe, but I'm new, so I'm not beating myself up too much.

---

Create a bigger temporary pointer, copy everything, push the new value, then delete the old pointer, and assign the temporary pointer to the class member.

Quote:

---

So, all of that said, did my declaration of plist within pop() mask or hide this->plist?

---

Yes. The rules of scope mean that the variable with the narrowest scope take precedence over variables with larger scope. The changes you made are not just so it stands out more to you. In this case, typing this->plist makes it explicit which plist you want to use, overriding the rules.

Agreed, you don't need to copy everything twice to resize the array. Just allocate one big enough and copy the existing item across, and then add the new item as well.
Reassigning pointers is easy.

Thanks to both of you for your help, it's appreciated.

Quote:

---

Originally Posted by whiteflags

then delete the old pointer, and assign the temporary pointer to the class member.

---

That didn't make sense at first until I thought about it. I was worried about leaving a dangling pointer and ending up with a memory leak, but after giving it some thought I see that the memory pointed to by the temporary pointer will be referred to by the pointer class member, Then the temporary pointer itself will be removed when the function call ends, cleaning everything up. Should the temporary pointer be explicitly deleted instead of relying on the end of the function call to remove it?

Quote:

---

Originally Posted by iMalc

Agreed, you don't need to copy everything twice to resize the array. Just allocate one big enough and copy the existing item across, and then add the new item as well.
Reassigning pointers is easy.

---

Just to make sure I have this right, the reason this is okay to do is because we're not losing track of any dynamically assigned memory. instead we're just passing the address around to a pointer that we know will free the memory later (class data member with destructor written to explicitly delete it)?

Quote:

---

Originally Posted by whiteflags

The changes you made are not just so it stands out more to you. In this case, typing this->plist makes it explicit which plist you want to use, overriding the rules.

---

Couldn't I could have left off the this-> portion as long as I didn't create another local plist variable? Something about an implicit this-> being used?

Basically, change line 177 (cstack.cpp, r17) from this:

Code:

---

this->plist = new int[new_stack_size];

---

to this:

Code:

---

plist = new int[new_stack_size];

---

This way I'm still correcting the mistake of creating a local variable to mask a variable in a higher scope, but I'm not explicitly using this->.

I still like using this-> and will try to keep in that habit because it seems to be useful in catching errors like the one I originally had, but I'm wondering if it's not required when referring to data members inside of a member function's code body.

Quote:

---

Originally Posted by deoren

TThat didn't make sense at first until I thought about it. I was worried about leaving a dangling pointer and ending up with a memory leak, but after giving it some thought I see that the memory pointed to by the temporary pointer will be referred to by the pointer class member, Then the temporary pointer itself will be removed when the function call ends, cleaning everything up. Should the temporary pointer be explicitly deleted instead of relying on the end of the function call to remove it?

Just to make sure I have this right, the reason this is okay to do is because we're not losing track of any dynamically assigned memory. instead we're just passing the address around to a pointer that we know will free the memory later (class data member with destructor written to explicitly delete it)?

---

The idea is that you allocate a new array that is big enough to hold the old data and the new data (with some size over preferably), and then copy everything over.

We now have two dynamically allocated arrays: the new and the old. Since we no longer need the old one, we must delete it. All that we new, we must delete. No exceptions. If you don't, you get a memory leak.
Now, we overwrite the old pointer's value with the new pointer's value. Hence, the old pointer's value (which is stored inside the class as a member) now points to the new memory area.

An even better solution than deleting is to use a smart pointer. Then we don't need to delete what we new.

Quote:

---

Couldn't I could have left off the this-> portion as long as I didn't create another local plist variable? Something about an implicit this-> being used?

Basically, change line 177 (cstack.cpp, r17) from this:

Code:

---

this->plist = new int[new_stack_size];

---

to this:

Code:

---

plist = new int[new_stack_size];

---

This way I'm still correcting the mistake of creating a local variable to mask a variable in a higher scope, but I'm not explicitly using this->.

I still like using this-> and will try to keep in that habit because it seems to be useful in catching errors like the one I originally had, but I'm wondering if it's not required when referring to data members inside of a member function's code body.

---

Yes, you can do that. So long as you do not create a new variable with an identical name to one stored as a class member, you do not need "this->". It is implicitly deduced by the compiler.
If you do shadow a member variable, then you do need "this->" to distinguish between the two variables. If you omit it, the local variable will be used instead.
Many programmers actually tend to prefer prefixing or subfixing (is that a word?) member variables to distinguish them from local ones. I always tend to add "m_" to all member variables.

Quote:

---

Originally Posted by deoren

Just to make sure I have this right, the reason this is okay to do is because we're not losing track of any dynamically assigned memory. instead we're just passing the address around to a pointer that we know will free the memory later (class data member with destructor written to explicitly delete it)?

---

You got the plot.

Quote:

---

Couldn't I could have left off the this-> portion as long as I didn't create another local plist variable? Something about an implicit this-> being used?

---

To be clear, "this->plist" is an unambigious way to write down the class member plist and do something with it. In most situations, omitting "this->" is your prerogative, but you introduced a local variable named plist in some function. That means that "plist" now refers to the local variable instead of the class member. It is a FAR better practice to make sure that class members have descriptive and unique names. I sometimes prefix "my", or a leading underscore (which can be troublesome, since the standard reserves all names _^ where ^ is a capital), but you can really use any little thing.

Quote:

---

Originally Posted by Elysia

subfixing

---

The word is suffix.

Thanks for the great replies!

Quote:

---

Originally Posted by deoren

Should the temporary pointer be explicitly deleted instead of relying on the end of the function call to remove it?

---

To answer that question, no, you should not do that. When I did so (cstack.cpp, r18) I was thinking of getting rid of the variable itself, but instead I was freeing up the memory I had just purposely reserved for the updated array (and now assigned to this->plist). My best guess is that I should allow the tmp_stack pointer to pass out of scope and be removed like an ordinary local variable (cstack.cpp, r19).

Quote:

---

Originally Posted by Elysia

Yes, you can do that. So long as you do not create a new variable with an identical name to one stored as a class member, you do not need "this->". It is implicitly deduced by the compiler.
If you do shadow a member variable, then you do need "this->" to distinguish between the two variables. If you omit it, the local variable will be used instead.
Many programmers actually tend to prefer prefixing or subfixing (is that a word?) member variables to distinguish them from local ones. I always tend to add "m_" to all member variables.

---

Good tip. I've seen the m_ prefix used quite a bit in the book I'm going through, and as my mistake has shown me, there is evidently a really good reason for doing so. :)

Quote:

---

Originally Posted by whiteflags

You got the plot.

---

Awesome, thanks for the confirmation.

Quote:

---

Originally Posted by deoren

To answer that question, no, you should not do that. When I did so (cstack.cpp, r18) I was thinking of getting rid of the variable itself, but instead I was freeing up the memory I had just purposely reserved for the updated array (and now assigned to this->plist). My best guess is that I should allow the tmp_stack pointer to pass out of scope and be removed like an ordinary local variable (cstack.cpp, r19).

---

Obviously you mustn't because you've just assigned tmp_stack to plist. So if you delete tmp_stack, plist will point to invalid memory now.
The right thing was done in (18), which deletes the old memory only and leaves the new intact.
Once again, you don't have to worry about this if you use smart pointers. I urge you to get familiar with them.

A smart pointer wouldn't be the recommendation from me. A container instead. But I also want deoren to follow his book. I suspect he's learning things more important than pointer practice right now. (Data structures.)

Both are very important. I'd suggest experimenting with both, just to get practice.
I doubt the book even mentions smart pointers, hence why I think the OP should experiment a little outside the book.

Smart pointers aren't even the right way to store arrays.

That's not right. All smart pointers support custom deleters. Plus boost has shared_array, I believe, they was made for arrays in mind.

Quote:

---

That's not right. All smart pointers support custom deleters

---

You're using the wrong tool for the job, and

Quote:

---

Plus boost has shared_array, I believe, they was made for arrays in mind.

---

this isn't a pointer anymore, it's a container (or if you really want to get to the bottom of it, an array is a sequence). I like consistent nomenclature. I think it's more understandable.

Quote:

---

Originally Posted by whiteflags

You're using the wrong tool for the job, and...

---

Then a pointer to hold a dynamic array is the wrong tool for the job.
Point is, the OP is doing just that, and hence, a smart pointer should relieve some of that burden.

Yes, the OP should prefer a container to a smart pointer to an array if possible, but then again, sometimes you have to allocate memory dynamically, and in all such cases, one should ask oneself if a smart pointer can be used, and if so, it should be used. That is the case right now.

Quote:

---

Originally Posted by Elysia

Once again, you don't have to worry about this if you use smart pointers. I urge you to get familiar with them.

---

Thanks for the suggestion. It's definitely on my To-Do list. I've heard too much about smart pointers being so much less error prone than traditional pointers that I would be doing myself a disservice not to.

Quote:

---

Originally Posted by whiteflags

A smart pointer wouldn't be the recommendation from me. A container instead. But I also want deoren to follow his book. I suspect he's learning things more important than pointer practice right now. (Data structures.)

---

Chapter 10 covers the STL, where vectors and other containers are taught. I can see the advantage of STL containers over arrays and the "native" C++ string type over C Strings.

I'm trying to stick close to the book to help provide structure and use other book resources to fill in gaps. I'm very prone to losing focus and "wandering off" in other directions, so my goal is to finish this book, then move on to others that I've purchased. I've also glanced through several of them and found that the progression should be a smooth one:

My list (in no particular order):

• C++ for Programmers
• Professional C++ (1e and 2e)
• C++ Primer Plus (5e and 6e)
• You Can Program in C++
• ...

If I don't watch myself, I will tend to collect programming books instead of actually use them. In a way it's too bad that I can't upload the knowledge like in the Matrix. It would sure beat carrying heavy books around. :P

Quote:

---

Originally Posted by Elysia

Both are very important. I'd suggest experimenting with both, just to get practice.
I doubt the book even mentions smart pointers, hence why I think the OP should experiment a little outside the book.

---

You're probably right (didn't see it in the index), but I will be exposed to it at some point as I go through the other books I mentioned. I'm looking forward to working with it.

Thanks again for everyone's help. Now, on to the next exercise. :)

Quote:

---

Point is, the OP is doing just that, and hence, a smart pointer should relieve some of that burden.

---

A smart pointer hacked with a deleter doesn't solve the problem either though -- you need to use new[] as well. Custom allocator maybe? I don't even know if they work like that to be honest. But you are fighting me more about this than you should be.

Quote:

---

Originally Posted by whiteflags

A smart pointer hacked with a deleter doesn't solve the problem either though -- you need to use new[] as well.

---

Err... that generally is less of a problem than not using a deleter to invoke delete[].

I never took the time to care about such a minute detail.

new[] pairs with delete[]
new pairs with delete

And I don't change that just because I'm using template magic and RAII.

Quote:

---

Originally Posted by whiteflags

And I don't change that just because I'm using template magic and RAII.

---

You shouldn't change that, which is precisely why if you do choose to use new[] with unique_ptr or shared_ptr, then you should provide a deleter to invoke delete[].

Since whiteflags still does not accept private messages, and since this might be of use to other people who wants to use custom deleters, here is an example on how to do it:

Code:

---

std::shared_ptr<int> test(new int[5], [](int* p) { delete [] p; });

---

Yep. That's it. That's all you need.

If you really need unqiue_ptr, then this should do the trick:

Code:

---

auto deleter = [](int* p) { delete [] p; };
std::unique_ptr<int, decltype(deleter)> test2(new int[5], deleter);

---

Quote:

---

Originally Posted by Elysia

If you really need unqiue_ptr, then this should do the trick:

Code:

---

auto deleter = [](int* p) { delete [] p; };
std::unique_ptr<int, decltype(deleter)> test2(new int[5], deleter);

---

---

I believe that you only need to write:

Code:

---

std::unique_ptr<int, std::default_delete<int[5]>> test2(new int[5]);

---

Actually from these 2 souces:
std::default_delete - Cppreference
std::unique_ptr - Cppreference

It should be enough to just write

Code:

---

std::unique_ptr<int[]> uptr(new int[10]);

---