abstract classes and virtual deconstructors assistance

A

Alden Pierre

Hello,

http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.7

As per the link above it's wise to have a virtual deconstructor when
creating an abstract class. Here is when I'm little confused. Am I
deleting the right object when I call the virtual deconstructor? I was
under impression when creating a class if I'm not specifically
allocating memory, do not implement deconstructor and let the default
take care of it for you. I have the following code:

-------------------------------------------------------------------------
File: person.h
-------------------------------------------------------------------------
#ifndef _PERSON_H
#define _PERSON_H

#include <iostream>
using std::string;

class Person
{
public:
Person( string, string );

void setName( string );
void setSSN( string );

const string getName( void ) const {return name;}
const string getSSN( void ) const {return ssn;}

virtual void print( void ) = 0;
virtual ~Person();

private:
string name;
string ssn;
};
#endif

-----------------------------------------------------------------------------------------
File: person.cpp
-----------------------------------------------------------------------------------------
#include "person.h"

using std::cout;
using std::endl;

Person::person( string in_name, string in_ssn )
{
setName( in_name );
setSSN( in_ssn );
}

Person::~Person()
{
delete this; // would this delete any derived class? return;
}

void Person::setName( string in_name )
{
name =in_name;
}

void Person::setSSN( string in_ssn )
{
ssn =in_ssn;
}

void Person::print( void )
{
cout << "Name: " << name << endl;
cout << "SSN: " << ssn << endl;
}
 
H

Howard

Alden Pierre said:
Hello,

http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.7

As per the link above it's wise to have a virtual deconstructor when
creating an abstract class. Here is when I'm little confused. Am I
deleting the right object when I call the virtual deconstructor?

You don't call the destructor. It is called for you when an object is
destroyed (either by calling delete on a pointer to the object, or when an
object goes out of scope).
I was under impression when creating a class if I'm not specifically
allocating memory, do not implement deconstructor and let the default take
care of it for you. I have the following code:

Generally, that's true. But, if you intend to inherit from the class (which
you'd have to do if it's abstract and you ever want to use it), AND if you
intend to delete an object of the inherited class via a pointer to the base
class, THEN your destructor needs to be virtual.

The question is, do you really need an abstract class here? And, will you
later be deleting an instance of an inherited class via a pointer to the
base class? By that I mean something like:

Person* person = new SpecialPerson(); // where SpecialPerson inherites from
Person
....
delete person;

-------------------------------------------------------------------------
File: person.h
-------------------------------------------------------------------------
#ifndef _PERSON_H
#define _PERSON_H

#include <iostream>
using std::string;

class Person
{
public:
Person( string, string );

void setName( string );
void setSSN( string );

const string getName( void ) const {return name;}
const string getSSN( void ) const {return ssn;}

virtual void print( void ) = 0;
virtual ~Person();

private:
string name;
string ssn;
};
#endif

-----------------------------------------------------------------------------------------
File: person.cpp
-----------------------------------------------------------------------------------------
#include "person.h"

using std::cout;
using std::endl;

Person::person( string in_name, string in_ssn )
{
setName( in_name );
setSSN( in_ssn );
}

Person::~Person()
{
delete this; // would this delete any derived class? return;

Don't do this. The destructor (this one) is called FOR YOU when the object
is deleted, so calling delete here would be redundant (and BAD).

And the return statement is also unneeded, (unless you're trying to return
before executing some action below it in the function, or if you're
returning a value). Just drop out of the function to exit.
}

void Person::setName( string in_name )
{
name =in_name;
}

void Person::setSSN( string in_ssn )
{
ssn =in_ssn;
}

void Person::print( void )
{
cout << "Name: " << name << endl;
cout << "SSN: " << ssn << endl;
}

Any reason you have a body for the print function here, when you've declared
it as pure virtual (by adding the "= 0" to the declaration in the class
definition above)?

-Howard
 
B

Ben Pope

Alden said:
Hello,

http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.7

As per the link above it's wise to have a virtual deconstructor when
creating an abstract class. Here is when I'm little confused. Am I
deleting the right object when I call the virtual deconstructor?

You probably do not want to call the virtual destructor explicitly. The
reason for the virtual destructor is so that when a derived object is
deleted through a base pointer, the right object (the derived one) is
destructed:

Person* p = new Derived();
delete p; // this deletes a "Derived" not a "Person".
I was
under impression when creating a class if I'm not specifically
allocating memory, do not implement deconstructor and let the default
take care of it for you.

If you are not allocating memory with the class (as opposed to
allocating memory FOR the class), then the default constructor is
appropriate, only if the class is not intended to be deleted though a base*.
I have the following code:

// identifiers beginning with an underscore followed by
// an uppercase character are reserved for the implementation
// Prefer e.g.,:
// FILE_PERSON_H
#define _PERSON_H

#include <iostream>
using std::string;

// Prefer not to put using declaration in header files
// fully qualify identifiers with their namespace.
// You are forcing everybody who uses this header file
// to introduce string into the global namespace
class Person
{
public:
Person( string, string );

// relating to the above prefer:
Person(std::string, std::string);
void setName( string );
void setSSN( string );

// Do you really need these set methods?
const string getName( void ) const {return name;}

// dont put void in an empty argument list, and the
// const is unnecessary as you return by value:
std::string getName() const { return name; }
const string getSSN( void ) const {return ssn;}

// similarly
virtual void print( void ) = 0;
virtual ~Person();

private:
string name;
string ssn;
};
#endif

-----------------------------------------------------------------------------------------

File: person.cpp
-----------------------------------------------------------------------------------------

#include "person.h"

using std::cout;
using std::endl;

Person::person( string in_name, string in_ssn )
{
setName( in_name );
setSSN( in_ssn );
}

// prefer initialisation list:
Person::person(string in_name, string in_ssn) :
name(in_name),
ssn(in_ssn)
{}
Person::~Person()
{
delete this; // would this delete any derived class? return;
}

// This is just unnecessary and possibly wrong:
// The following will suffice and do the right thing:
Person::~Person() {}
void Person::setName( string in_name )
{
name =in_name;
}

// I still think setName is not necessary, how often
// does the name change after construction?
void Person::setSSN( string in_ssn )
{
ssn =in_ssn;
}

// Similarly
void Person::print( void )
{
cout << "Name: " << name << endl;
cout << "SSN: " << ssn << endl;
}

This function is unnecessary as a member, the information can be
obtained from the public interface, I would write:

std::eek:stream& operator<<(std::eek:stream& os, const Person& p) {
os << "Name: " << p.getName() << "\n";
os << "SSN: " << p.getSSN() << "\n";
return os;
}

And then use it like:

int main() {
Person person("Ben", "foo");
std::cout << person << std::endl;
}

Ben Pope
 
H

Howard

Alden Pierre said:
Hello,

http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.7

As per the link above it's wise to have a virtual deconstructor when
creating an abstract class. Here is when I'm little confused. Am I
deleting the right object when I call the virtual deconstructor? I was
under impression when creating a class if I'm not specifically allocating
memory, do not implement deconstructor and let the default take care of it
for you. I have the following code:

Another point that occurs to me: suppose you have a DerivedPerson class,
derived from Person (naturally). It may be that DerivePerson DOES allocate
memory which it needs to clean up. In that case, it will need a destructor
to do that in, right?

Then, if you're referring to an instance of that DerivePerson with a Person*
pointer, you'll have to have the virtual destructor in your Person class in
order for the correct destructor to be called.

And since you can't guarantee in advance (well, MAYBE you can, but
anyway...) that you will never need to delete an object dervived from Person
via a Person* pointer, it's better to assume that someday you MIGHT, so
ading a virtual destructor to your base class, even if it's empty (and even
if the base class isn't abstract), is a good idea.

-Howard
 
A

Alden Pierre

Howard said:
You don't call the destructor. It is called for you when an object is
destroyed (either by calling delete on a pointer to the object, or when an
object goes out of scope).


Generally, that's true. But, if you intend to inherit from the class (which
you'd have to do if it's abstract and you ever want to use it), AND if you
intend to delete an object of the inherited class via a pointer to the base
class, THEN your destructor needs to be virtual.

The question is, do you really need an abstract class here? And, will you
later be deleting an instance of an inherited class via a pointer to the
base class? By that I mean something like:

Person* person = new SpecialPerson(); // where SpecialPerson inherites from
Person
...
delete person;



Don't do this. The destructor (this one) is called FOR YOU when the object
is deleted, so calling delete here would be redundant (and BAD).

And the return statement is also unneeded, (unless you're trying to return
before executing some action below it in the function, or if you're
returning a value). Just drop out of the function to exit.


Any reason you have a body for the print function here, when you've declared
it as pure virtual (by adding the "= 0" to the declaration in the class
definition above)?

-Howard

Yes, this is by request through my prof. The classes which derive from
person is the following: student, faculty, and admin. They all
implement there on print function as well. Student will print name,
ssn, and major. Faculty will print name, ssn, and dept. Last, but not
least, admin will print name, ssn, title, and boss. I guess the reason
behind this request is not to type too much. So within the derived
class implementation file, I would have something like this as my print
function: Person::print() and then cout << data-member.

Regards,
Alden
 
A

Alden Pierre

Ben said:
You probably do not want to call the virtual destructor explicitly. The
reason for the virtual destructor is so that when a derived object is
deleted through a base pointer, the right object (the derived one) is
destructed:

Person* p = new Derived();
delete p; // this deletes a "Derived" not a "Person".


If you are not allocating memory with the class (as opposed to
allocating memory FOR the class), then the default constructor is
appropriate, only if the class is not intended to be deleted though a
base*.


// identifiers beginning with an underscore followed by
// an uppercase character are reserved for the implementation
// Prefer e.g.,:
// FILE_PERSON_H


// Prefer not to put using declaration in header files
// fully qualify identifiers with their namespace.
// You are forcing everybody who uses this header file
// to introduce string into the global namespace


// relating to the above prefer:
Person(std::string, std::string);


// Do you really need these set methods?


// dont put void in an empty argument list, and the
// const is unnecessary as you return by value:
std::string getName() const { return name; }


// similarly


// prefer initialisation list:
Person::person(string in_name, string in_ssn) :
name(in_name),
ssn(in_ssn)
{}


// This is just unnecessary and possibly wrong:
// The following will suffice and do the right thing:
Person::~Person() {}


// I still think setName is not necessary, how often
// does the name change after construction?


// Similarly


This function is unnecessary as a member, the information can be
obtained from the public interface, I would write:

std::eek:stream& operator<<(std::eek:stream& os, const Person& p) {
os << "Name: " << p.getName() << "\n";
os << "SSN: " << p.getSSN() << "\n";
return os;
}

And then use it like:

int main() {
Person person("Ben", "foo");
std::cout << person << std::endl;
}

Ben Pope

Ben, thanks, for your critique while learning c++ I want to develop a
good coding style so that others are not frustrated by the way I code.

Regards,
Alden
 
C

Cy Edmunds

Alden Pierre said:
Yes, this is by request through my prof. The classes which derive from
person is the following: student, faculty, and admin. They all implement
there on print function as well. Student will print name, ssn, and major.
Faculty will print name, ssn, and dept. Last, but not least, admin will
print name, ssn, title, and boss. I guess the reason behind this request
is not to type too much. So within the derived class implementation file,
I would have something like this as my print function: Person::print() and
then cout << data-member.

Regards,
Alden

Honestly, this is a terrible design. In partitioning software functions you
should frequently ask yourself, "what does X have to do with Y?" In this
case, what does personal data such as name and ssn have to do with printing
anything? Absolutely nothing.

Stroustrop talks about abstract data types such as:

class Printable
{
public:
virtual void print() = 0; // nothing but pure virtual functions and...
~Printable() {} // a do-nothing virtual destructor
};

and concrete data types such as

class Person
{
private:
std::string m_name;
std::string m_ssn;
public:
// no virtual functions
Person(std::string const &i_name, std::string const &i_ssn) :
m_name(i_name), m_ssn(i_ssn) {}
std::string const &name() const {return m_name;}
std::string const &ssn() const {return m_ssn;}
};

Then you can print any way you want without using Printable in any way:

std::eek:stream &
print_my_way(std::eek:stream &os, Person const &p)
{
os << p.name() << ' ' << p.ssn() << '\n';
return os;
}

std::eek:stream &
print_your_way(std::eek:stream &os, Person const &p) {...}

This gives just as much flexibility as virtual functions without breaking
encapsulation in Person. (All code is unchecked and typed quickly over lunch
hour.)

I generally only use abstract data types if I want to create a collection of
smart pointers to a common base class -- in other words a collection of
things that have a common interface but different actual types.

Cy
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,995
Messages
2,570,230
Members
46,818
Latest member
Brigette36

Latest Threads

Top