3

I was reading through Chapter 13 of the C++ Primer Plus book. It had an example that dealt with using inheritance with dynamic memory allocation, copy constructors and overloading the = operator.

The base class, which is called baseDMA has a private pointer-to-char that uses new in the constructor.

class baseDMA
{
private:
   char * label;

...
};

baseDMA::baseDMA(const char * l)
{
    label = new char[std::strlen(l) + 1];
    std::strcpy(label, l);
    ...
}

Now when overloading the = operator, we delete the label pointer because we will be assigning it to a new value and it will point to a new location. If we don't delete it then we will not be able to do so later because the pointer will point to something different now and the old location that was pointed to by this pointer is not deleted and also has nothing pointed to it now (this is how the author explains it in a different chapter)

This is the overloaded = operator for the base class:

baseDMA & baseDMA::operator=(const baseDMA & rs)
{
   if (this == &rs)
      return *this;
   delete [] label;
   label = new char[std::strlen(rs.label) + 1];
   std::strcpy(label, rs.label);
   return *this;
}

Next the author defines a derived class called hasDMA, which also uses new for a pointer-to-char that he he defines as the following:

class hasDMA :public baseDMA
{
private:
    char * style;
    ...
};

hasDMA::hasDMA(const char * s, const char * l)
: baseDMA(l)
{
    style = new char[std::strlen(s) + 1];
    std::strcpy(style, s);
}

Now the part the confuses me a little, is that when the author overloads the = operator for the derived class, he doesn't seem to delete [] style before giving it a new value, just as he did with label from the base class. This is how the author did the overloaded = operator for the derived class:

hasDMA & hasDMA::operator=(const hasDMA & hs)
{
    if (this == &hs)
       return *this;
    baseDMA::operator=(hs); // copy base portion
    //no delete [] style
    style = new char[std::strlen(hs.style) + 1];
    std::strcpy(style, hs.style);
    return *this;
} 

What is the reason for not freeing the memory pointed to by style just as we freed the memory pointed to by label from the base class before assigning it a new value?

Thanks in advance

R. Martinho Fernandes
  • 228,013
  • 71
  • 433
  • 510
Kakalokia
  • 3,191
  • 3
  • 24
  • 42

1 Answers1

6

The reason is because the author made a mistake. This is an excellent example of how you should not ever manage your own memory- he should be using std::vector to manage his memory. He didn't, and as a result, his code was very wrong, and that's exactly the way your code is going to go if you imitate him.

In addition, he uses the seriously outdated self-assignment-check no-longer-an-idiom, and no copy-and-swap.

In short, get a new book. This is all bad.

Puppy
  • 144,682
  • 38
  • 256
  • 465
  • He's obviously not going to use `std::string` when he's reinventing a string class. – Puppy Feb 23 '13 at 19:08
  • I see. Well in all fairness to the author, it's a very introductory book, and he has to take you step by step instead of throwing a lot of things at you at once. I guess he might introduce the things you mentioned in the other chapters that I haven't read it. – Kakalokia Feb 23 '13 at 19:08
  • 1
    We should remove this book from pole position under "Beginner/Introductory" on http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list if we want to continue proclaiming that [tag:c++-faq] questions herald from some self-ordained expertry. – Lightness Races in Orbit Feb 23 '13 at 19:09
  • @Ali just for clarity, which version do you have? – R. Martinho Fernandes Feb 23 '13 at 19:09
  • 3
    @AliAlamiri: These things *should be in the introduction*. They are not advanced features. Screwing around with your own memory is far more advanced. – Puppy Feb 23 '13 at 19:09
  • 2
    Is it *C++ Primer* or *C++ Primer **Plus***? –  Feb 23 '13 at 19:09
  • @R.MartinhoFernandes 5th :-s – Kakalokia Feb 23 '13 at 19:10
  • @Lightness: Depends on the versions involved- if he has one from 1990 and the newest are better, then there might be cause for clarification rather than removal. – Puppy Feb 23 '13 at 19:10
  • 4
    @AliAlamiri: Primer and Primer Plus are *very* different. Primer Plus is legendarily extremely bad- they're not even from the same author, Primer PLus is written by Bullschildt. You've been duped. – Puppy Feb 23 '13 at 19:11
  • @AliAlamiri that book is a good source for energy. Burn it as soon as possible and buy [a good one](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). –  Feb 23 '13 at 19:11
  • Well I'm kinda saddened by all this. Been spending quite some time reading the book. (Try not to cry Ali) – Kakalokia Feb 23 '13 at 19:14
  • 5
    I don't understand why you don't just answer the question, but grumble about the book. The grumbling should have been a comment, if at all. *"... he should be using std::vector to manage his memory. He didn't, and as a result, his code was very wrong, and that's exactly the way your code is going to go"*. That's the most funny and sad thing I heard today. Doing your own string for learning purposes is a very good thing. *In addition, he uses the seriously outdated self-assignment-check no-longer-an-idiom, and no copy-and-swap.* There is nothing wrong with that. The former is not "outdated". – Johannes Schaub - litb Feb 23 '13 at 19:20
  • 1
    in additon to the puppy and the lion's good remarks, consider always defaulting to implement assignment in terms of copy construction, not assignment on its own. it's simpler, and generally more exception safe. – Cheers and hth. - Alf Feb 23 '13 at 19:21
  • 1
    By the way just to mention, the book is written by Stephen Prata, not Bullschidt – Kakalokia Feb 23 '13 at 19:22
  • @JohannesSchaub-litb: i agree with your remarks about the misperception of the purpose of the book's example, that perhaps deadmg is overstating things here (not considering that the example can be intentionally low-level ungood). however, as i just commented, as a default for what to do, it's not a good idea to implement assignment on its own. the copy-and-swap idiom is simple, generally efficient and exception safe. – Cheers and hth. - Alf Feb 23 '13 at 19:23
  • 1
    @Johannes: self-assignment-check is no longer idiomatic. The grumbling *is* the answer to the question. The question is "Why does this book contain a defect?" and the answer is "Because it's written by a moron who has no idea what he's doing.". – Puppy Feb 23 '13 at 19:23
  • hm, i think i never related my exerience with some c++ code used as "scripting" a geographic system app, where copy construction was implemented in terms of assignment. boy was that ugly! the code also generally had umpteen entry points to anything, you never knew where or when anything was initilized or updated. they sold this thingy to the dutch army. – Cheers and hth. - Alf Feb 23 '13 at 19:26
  • @JohannesSchaub-litb No, it **is** outdated because it’s not transactionally exception safe (if `new` throws the object should remain unchanged). – Konrad Rudolph Feb 25 '13 at 09:30
  • @konrad there are libraries and programs that do not need that safety. just because you can do something doesnt mean you have to do it. coding exception safe libraries is difficult and development time consuming for lot of people. as an excellent example, consider boost::variant assignment operator. – Johannes Schaub - litb Feb 26 '13 at 10:08
  • such libraries, including boost::optional, dont have an O1 swap so the copy and swap idiom is not applicable. – Johannes Schaub - litb Feb 26 '13 at 10:09
  • @Johannes But we’re not talking about such a library here. Copy&swap is applicable here. – Konrad Rudolph Feb 26 '13 at 10:13
  • @KonradRudolph again, something being applicable doesn't mean that something must be used. you could label any book having a simple "string" class as using "outdated" mechanism for their operator+ just because they don't use expression templates for their operators or just because they don't use move constructors. self-assignment checks are very simple and serve their purpose just fine, and in no way "outdated". copy+swap can be used in advanced chapters about exception safety. – Johannes Schaub - litb Feb 26 '13 at 18:36