I need help with an inheritance issue. I am implementing a sort of
symbolic calculus code. By the way, is the site www.ginac.de closed ?
Where can I find a mirror ?
Back to my code. I have a base class "Expression" and derived classes
"ExpressionSum", "ExpressionProduct", "ExpressionPower" and so on. For
instance, xy+yz+z is an ExpressionSum of two ExpressionProduct and an
Expression. The class ExpressionSum is implemented as a list of terms
to sum up. These terms should be any other Expressions (Products,
Powers, or mere Expressions). The problem is, when I call
ExpressionSum::add(expr), expr being an ExpressionProduct, it looks
like expr is first converted to a mere Expression and then added to
the list. This is different from what I want. I want a list of
Expressions, some of which should behave like Products, other like
Powers, etc.
I hope I made myself clear. I include below a small piece of code.
Thank you. Cristian Barbarosie http://cmaf.ptmat.fc.ul.pt/~barbaros/
#include <iostream>
#include <list>
using namespace std;
class Expression {
public:
string nname;
Expression ()
{ cout << "constructor Expression, no name provided" << endl;}
Expression (string n)
{ cout << "constructor Expression, name " << n << endl;
nname = n; }
virtual string name()
{ cout << "method Expression::name"<< endl; return nname; }
};
class ExpressionProduct : public Expression {
public:
list<Expression> factors;
ExpressionProduct ()
{cout << "constructor ExpressionProduct, no arguments" << endl;}
ExpressionProduct (const Expression &f)
{ cout << "constructor ExpressionProduct, one argument" << endl;
factors.push_back(f); }
ExpressionProduct (const Expression &f, const Expression &g)
{ cout << "constructor ExpressionProduct, two arguments" << endl;
factors.push_back(f);
factors.push_back(g); }
string name()
{ cout << "method ExpressionProduct::name"<< endl;
list<Expression>::iterator i=factors.begin();
if (i==factors.end()) return "1";
string n;
for (;i!=factors.end();i++) n = n + i->name();
return n; }
};
class ExpressionSum : public Expression {
public:
list<Expression> terms;
ExpressionSum ()
{cout << "constructor ExpressionSum, no arguments" << endl;}
void add (Expression &m)
{ cout << "adding" << endl; terms.push_back(m); }
string name()
{ cout << "method ExpressionSum::name"<< endl;
list<Expression>::iterator i=terms.begin();
if (i==terms.end()) return "0";
string n=i->name();
i++ ;
for (;i!=terms.end();i++)
n = n + '+' + i->name();
return n; }
};
int main () {
Expression x("x"), y("y"), z("z");
ExpressionProduct xy(x,y), yz(y,z);
cout << xy.name() << endl;
ExpressionSum p;
p.add(xy); p.add(yz); p.add(z);
cout << p.name() << endl;
}
No
> Back to my code. I have a base class "Expression" and derived classes
> "ExpressionSum", "ExpressionProduct", "ExpressionPower" and so on. For
> instance, xy+yz+z is an ExpressionSum of two ExpressionProduct and an
> Expression. The class ExpressionSum is implemented as a list of terms
> to sum up. These terms should be any other Expressions (Products,
> Powers, or mere Expressions). The problem is, when I call
> ExpressionSum::add(expr), expr being an ExpressionProduct, it looks
> like expr is first converted to a mere Expression and then added to
> the list.
Yes, you stores Expression values.
You should store Expression pointers which raises the problem of deep
copy. It is usually solved by implementing a 'virtual Expression* clone
()' operation.
Another alternative is to use the Envelope/Letter idiom.
> This is different from what I want. I want a list of
> Expressions, some of which should behave like Products, other like
> Powers, etc.
>
> I hope I made myself clear.
Yes
> I include below a small piece of code.
[snip]
> class Expression {
[snip]
> class ExpressionProduct : public Expression {
[snip]
> class ExpressionSum : public Expression {
> public:
> list<Expression> terms;
Here you have slicing of your objects.
If you make Expression virtual pure, you won't be able to compile your
code.
> ExpressionSum ()
> {cout << "constructor ExpressionSum, no arguments" << endl;}
> void add (Expression &m)
> { cout << "adding" << endl; terms.push_back(m); }
> string name()
> { cout << "method ExpressionSum::name"<< endl;
> list<Expression>::iterator i=terms.begin();
> if (i==terms.end()) return "0";
> string n=i->name();
> i++ ;
> for (;i!=terms.end();i++)
> n = n + '+' + i->name();
> return n; }
>
> };
>
> int main () {
> Expression x("x"), y("y"), z("z");
> ExpressionProduct xy(x,y), yz(y,z);
> cout << xy.name() << endl;
> ExpressionSum p;
> p.add(xy); p.add(yz); p.add(z);
> cout << p.name() << endl;
>
> }
--
Michael
I agree that this is the problem. Your lists are of type Expression,
so only Expression objects will live in the list. When you push_back
onto the list, Expression objects are created. Since an Expression
object doesn't have the members "factors" or "terms", you lose this
information when the Expression object is created (from the
ExpressionSum or ExpressionProduct object).
One thing you might try is to make your lists hold pointers to
Expression objects. This way the list can work with Expression
objects or one of it's derived types (ExpressionSum or
ExpressionProduct). Keep in mind, that since you are dealing with a
list of pointers, you have to make sure that the objects pointed to
continue to exist throughout the life of the list.
Here is an example with pointers, although smart pointers would be a
better idea. (Also, I put in some more additional const correctness.)
#include <iostream>
#include <list>
using namespace std;
class Expression {
public:
string nname;
Expression ()
{ cout << "constructor Expression, no name provided" << endl;}
Expression (string n)
{ cout << "constructor Expression, name " << n << endl;
nname = n; }
virtual string name() const
{ cout << "method Expression::name"<< endl; return nname; }
};
class ExpressionProduct : public Expression {
public:
list<const Expression *> factors;
ExpressionProduct ()
{cout << "constructor ExpressionProduct, no arguments" << endl;}
ExpressionProduct (const Expression &f)
{ cout << "constructor ExpressionProduct, one argument" << endl;
factors.push_back(&f); }
ExpressionProduct (const Expression &f, const Expression &g)
{ cout << "constructor ExpressionProduct, two arguments" << endl;
factors.push_back(&f);
factors.push_back(&g); }
string name() const
{ cout << "method ExpressionProduct::name"<< endl;
list<const Expression *>::const_iterator i=factors.begin();
if (i==factors.end()) return "1";
string n;
for (;i!=factors.end();i++) n = n + (*i)->name();
return n; }
};
class ExpressionSum : public Expression {
public:
list<const Expression *> terms;
ExpressionSum ()
{cout << "constructor ExpressionSum, no arguments" << endl;}
void add (const Expression &m)
{ cout << "adding" << endl; terms.push_back(&m); }
string name() const
{ cout << "method ExpressionSum::name"<< endl;
list<const Expression *>::const_iterator i=terms.begin();
if (i==terms.end()) return "0";
string n=(*i)->name();
i++ ;
for (;i!=terms.end();i++)
n = n + '+' + (*i)->name();
return n; }
};
int main () {
Expression x("x"), y("y"), z("z");
ExpressionProduct xy(x,y), yz(y,z);
cout << xy.name() << endl;
ExpressionSum p;
p.add(xy); p.add(yz); p.add(z);
cout << p.name() << endl;
return 0;
}
> > Back to my code. I have a base class "Expression" and derived
> > classes "ExpressionSum", "ExpressionProduct", "ExpressionPower" and
> > so on. For instance, xy+yz+z is an ExpressionSum of two
> > ExpressionProduct and an Expression. The class ExpressionSum is
> > implemented as a list of terms to sum up. These terms should be any
> > other Expressions (Products, Powers, or mere Expressions). The
> > problem is, when I call ExpressionSum::add(expr), expr being an
> > ExpressionProduct, it looks like expr is first converted to a mere
> > Expression and then added to the list.
> Yes, you stores Expression values. You should store Expression
> pointers which raises the problem of deep copy. It is usually solved
> by implementing a 'virtual Expression* clone ()' operation.
> Another alternative is to use the Envelope/Letter idiom.
Which just hides the cloning. If Expression objects are immutable
(and
they probably should be), then a much simpler solution would be to use
reference semantics (i.e. pointers) everywhere. Depending on the
application, this might even be a case where shared_ptr is
appropriate---an expression tree is just that, a tree, and can't have
cycles. (There are other solutions, with better performance, but they
generally require more work.)
> [snip]
> > class Expression {
> [snip]
> > class ExpressionProduct : public Expression {
> [snip]
> > class ExpressionSum : public Expression {
> > public:
> > list<Expression> terms;
> Here you have slicing of your objects.
> If you make Expression virtual pure, you won't be able to compile your
> code.
General rule: assignment and external copy don't work well with
inheritance. (In my own code, I've gradually been introducing a
PolymorphicObject base class, with a virtual destructor and a private
copy constructor and assignment operator. With the rule that classes
designed to be used polymorphically should inherit from
PolymorphicObject.)
--
James Kanze
I do not understand why you said that "assignment and external copy
don't work well with inheritance."
Can you provide a simple example how your code looks like using
PolymorphicObject?
And handles automatically the deletion in containers and some hard
cases of exception safety.
I also though the Envelope/Letter idiom would be a good choice for
symbolic calculus code because type promotion is handled seamlessly.
> If Expression objects are immutable
> (and
> they probably should be), then a much simpler solution would be to use
> reference semantics (i.e. pointers) everywhere. Depending on the
> application, this might even be a case where shared_ptr is
> appropriate---an expression tree is just that, a tree, and can't have
> cycles. (There are other solutions, with better performance, but they
> generally require more work.)
Yes. Or something like boost's intrusive_ptr<>; AMA it wouldn't be too
onerous in development.
[snip]
--
Michael
Mainly it has to do with C++ variables directly being of the size of the
statically known type and directly containing an object of that type, instead of
just being pointers[1] as in Java and C# and like languages.
When sizeof(Derived) > sizeof(Base) this means that
Base o = Derived();
performs a /slice/ of the Derived object; 'o' contains only the Base stuff of
that original object.
Additionally, the copy is now a Base, so any overriding of functionality in
Derived is lost.
Even worse, consider
Derived o;
Base& b = o;
b = Base();
Perhaps Base has a person's name and Derived additional has the person's
birth-year, then the above changes the 'o' name without updating the birth-year,
yielding a Derived instance with inconsistent information.
For a PolymorphicObject base class like James mentioned you therefore generally
want to introduce two restrictions, and support one main functionality:
* Inhibit client code "slice" copying.
This is done by making the assignment operator private and the
copy constructor protected. James wrote "private" copy constructor
but that's a bit impractical. For you want to allow derived classes
to be clonable, and cloning is best expressed in terms of internal
copy construction.
* Make sure that objects can only be created dynamically.
The reasonable way is to make the destructor protected.
* Force use of smart pointer.
James relies on garbage collection so he probably doesn't do this,
but there are two aspects: ensuring that any newly created object's
raw pointer is immediately stored in a smart pointer, and ensuring
that only the smart pointer class has access to destroy an object.
One way to do the first it is to overload the class' allocation function
(operator new) so that any direct 'new' expression would be overly
complicated. For C++98 then provide a macro that supplies the
requisite magic incomprehensible expression and ensures the pointer
is immediately wrapped in a smart pointer, before client code can
get at it. For C++0x I think the improved support for argument
forwarding makes the macro unnecessary. Anyways, one way to do the
second is to make destructor protected (which you'd do anyway for
the bullet point above), and grant friendship to the smart pointer.
Cheers & hth.,
- Alf
Notes:
[1] Even though Java /programmers/ often think that Java doesn't have pointers,
the Java language specification uses that (correct) terminology. C++ programmers
are more conscious of the low level and formal stuff, because they have to be:
C++ is much more complicated... So, I'm not confusing terms here.
On Dec 15, 8:43 pm, "John H." <oldman_fromt...@yahoo.com> wrote:
> [...] Your lists are of type Expression,
> so only Expression objects will live in the list. When you push_back
> onto the list, Expression objects are created.
Could you please elaborate a little bit on that ?
I am confused because I have output instructions in each and every
constructor. So, if new Expression objects were created, I should
obtain messages on the screen before or after each "add" operation.
I include below the output of the code in my first message
(under Debian 4.3.3-3)
constructor Expression, name x
constructor Expression, name y
constructor Expression, name z
constructor Expression, no name provided
constructor ExpressionProduct, two arguments
constructor Expression, no name provided
constructor ExpressionProduct, two arguments
method ExpressionProduct::name
method Expression::name
method Expression::name
xy
constructor Expression, no name provided
constructor ExpressionSum, no arguments
adding
adding
adding
method ExpressionSum::name
method Expression::name
method Expression::name
method Expression::name
++z
You can have a problem hereafter.
> method Expression::name
> method Expression::name
> method Expression::name
If your code was correct, the output should have been:
method ExpressionProduct::name
method ExpressionProduct::name
method Expression::name
It is because your container stores only the Expression part of
ExpressionProduct.
--
Michael
Ok, this explains it :)
[snip]
> For a PolymorphicObject base class like James mentioned you therefore
> generally want to introduce two restrictions, and support one main
> functionality:
>
> * Inhibit client code "slice" copying.
> This is done by making the assignment operator private and the
> copy constructor protected. James wrote "private" copy constructor
> but that's a bit impractical. For you want to allow derived classes
> to be clonable, and cloning is best expressed in terms of internal
> copy construction.
>
The assignment operator is private and not implemented, or just private?
> * Make sure that objects can only be created dynamically.
> The reasonable way is to make the destructor protected.
>
But if you make the constructor protected, and leave the destructor
public, then you do not need next point. Or, am I missing something?
> > General rule: assignment and external copy don't work well with
> > inheritance. (In my own code, I've gradually been introducing a
> > PolymorphicObject base class, with a virtual destructor and a
> > private copy constructor and assignment operator. With the rule
> > that classes designed to be used polymorphically should inherit from
> > PolymorphicObject.)
> I do not understand why you said that "assignment and external copy
> don't work well with inheritance."
In the case of polymorphic objects, the "value" of an object includes
its type, and you cannot change the type of a declared object. Given
something like:
Base* p1 = new Derived1;
Base* p2 = new Derived2;
*p1 = *p2;
What should this mean? The same thing occurs with external copy: you
have to specify the name of the constructed type, something like:
Base o( *p1 );
This constructs a Base, not a Derived1.
> Can you provide a simple example how your code looks like using
> PolymorphicObject?
class Expression : public PolymorphicObject
{
// ...
};
That's all that's necessary.
--
James Kanze
There's usually no need to implement it.
I can't imagine why one need it, but it's always like, someone's going to need
that anyway.
So the answer is "it depends", but in general, no implementation.
>> * Make sure that objects can only be created dynamically.
>> The reasonable way is to make the destructor protected.
>>
>
> But if you make the constructor protected, and leave the destructor
> public, then you do not need next point. Or, am I missing something?
Yeah. First of all you need the destructor protected anyway, to stop the
long-fingered client code programmer from doing 'delete &*smart_ptr'. Or more
indirectly. And secondly, with M classes with on average N constructors each
then for protected constructors you need M*N factory functions, while with
destructor protected you need, uh, none. Or just a single common one, if
wrapping of new expression is regarded as a factory function. :-)
>> * Force use of smart pointer.
>> James relies on garbage collection so he probably doesn't do this,
>> but there are two aspects: ensuring that any newly created object's
>> raw pointer is immediately stored in a smart pointer, and ensuring
>> that only the smart pointer class has access to destroy an object.
>> One way to do the first it is to overload the class' allocation
>> function
>> (operator new) so that any direct 'new' expression would be overly
>> complicated. For C++98 then provide a macro that supplies the
>> requisite magic incomprehensible expression and ensures the pointer
>> is immediately wrapped in a smart pointer, before client code can
>> get at it. For C++0x I think the improved support for argument
>> forwarding makes the macro unnecessary. Anyways, one way to do the
>> second is to make destructor protected (which you'd do anyway for
>> the bullet point above), and grant friendship to the smart pointer.
Your explanation seems eerily similar to the concept of Expression
Trees as a part of the LINQ infrastructure in C#. I know its slightly
off topic but you can look at the System.Linq.Expressions hierarchy
and get an idea.
http://msdn.microsoft.com/en-us/library/bb397951.aspx
http://msdn.microsoft.com/en-us/library/system.linq.expressions.aspx
> > James Kanze wrote:
> >> General rule: assignment and external copy don't work well with
> >> inheritance. (In my own code, I've gradually been introducing a
> >> PolymorphicObject base class, with a virtual destructor and a
> >> private copy constructor and assignment operator. With the rule
> >> that classes designed to be used polymorphically should inherit
> >> from PolymorphicObject.)
> > I do not understand why you said that "assignment and external copy
> > don't work well with inheritance."
> Mainly it has to do with C++ variables directly being of the size of
> the statically known type and directly containing an object of that
> type, instead of just being pointers[1] as in Java and C# and like
> languages.
> When sizeof(Derived) > sizeof(Base) this means that
> Base o = Derived();
> performs a /slice/ of the Derived object; 'o' contains only the Base
> stuff of that original object.
Not just when the sizes are different. The fact that the derived type
can be bigger than the base type (and that the compiler needs to know
the size static and member variables) may be the motivation here, but
the important point is that an object in C++ (or in Java) cannot
change
its type, and that variables in C++ do have object type (rather than
reference type, as in Java). And slicing occurs even if the sizes are
the same---o has type Base.
In many cases (and almost certainly in his), the base class should be
abstract, which guarantees no slicing (since you can't have instances
of
an abstract type).
> Additionally, the copy is now a Base, so any overriding of
> functionality in Derived is lost.
> Even worse, consider
> Derived o;
> Base& b = o;
> b = Base();
> Perhaps Base has a person's name and Derived additional has the
> person's birth-year, then the above changes the 'o' name without
> updating the birth-year, yielding a Derived instance with inconsistent
> information.
> For a PolymorphicObject base class like James mentioned you therefore
> generally want to introduce two restrictions, and support one main
> functionality:
> * Inhibit client code "slice" copying.
> This is done by making the assignment operator private and the
> copy constructor protected. James wrote "private" copy constructor
> but that's a bit impractical. For you want to allow derived classes
> to be clonable, and cloning is best expressed in terms of internal
> copy construction.
Oops. You're right, if you want to support cloning, *and* the base
class has state, you'll need a protected copy constructor. (Again, in
the most common scenario, and the one that should be used here, the
base
class will be an "interface": abstract and without state. And the
derived classes copy constructors can simply ignore it.)
> * Make sure that objects can only be created dynamically.
> The reasonable way is to make the destructor protected.
In practice, I suspect that this may be overkill if the base class is
abstract. Except for construction, client code will only use the base
class. And there's no way they can accidentally declare a variable
with
the type of the base class.
Also, in the very special case of Expression (and maybe one or two
others), there are scenarios where you don't want dynamic allocation
(despite polymorphism). The mode today is to use templates for
compile
time expression evaluation, but in the past, a virtual hierarchy based
on expression did the trick just as well---provided all of the
instances
of the derived class were temporaries on the stack, so that the
compiler
knew the actual types and could inline the virtual functions. (But as
I
said, that's a very special case.)
> * Force use of smart pointer.
> James relies on garbage collection so he probably doesn't do
> this, but there are two aspects: ensuring that any newly created
> object's raw pointer is immediately stored in a smart pointer,
> and ensuring that only the smart pointer class has access to
> destroy an object.
It depends on context, and I don't use garbage collection everywhere.
(Only when I can.) And as I mentionned somewhere, in this particular
case, boost::shared_ptr is a more than adequate solution. Probably
slower than the obvious alternatives (including garbage collection),
but
probably fast enough, and certainly a lot simpler to implement, unless
you're already using garbage collection.
For this case. In many other cases (entity objects, etc.), you don't
really want smart pointers except temporarily (if then---often,
everything that's necessary to accomplish before the smart pointer
gives
up ownership can be done in the constructor).
> One way to do the first it is to overload the
> class' allocation function (operator new) so that any direct
> 'new' expression would be overly complicated. For C++98 then
> provide a macro that supplies the requisite magic
> incomprehensible expression and ensures the pointer is
> immediately wrapped in a smart pointer, before client code can
> get at it.
You don't need to be that complicated. Just make the constructors
private, and provide a factory function which returns a smart pointer.
Something like:
class Expression
{
Expression( Expression const& );
Expression& operator=(Expression const& );
protected:
Expression() {}
~Expression() {}
public:
typedef boost::shared_ptr< Expression > Ptr;
virtual double value() const = 0;
};
class AddExpression : public Expression
{
Ptr lhs;
Ptr rhs;
AddExpression( Ptr lhs, Ptr rhs )
: m_lhs( lhs )
, m_rhs( rhs )
{
}
public:
static Ptr create( Ptr lhs, Ptr rhs )
{
return Ptr( new AddExpression( lhs, rhs ) );
}
virtual double value() const
{
return lhs->value() + rhs->value();
}
};
That should go a long way to offering the protection you want.
--
James Kanze
[...]
> > > Yes, you stores Expression values. You should store Expression
> > > pointers which raises the problem of deep copy. It is usually solved
> > > by implementing a 'virtual Expression* clone ()' operation.
> > > Another alternative is to use the Envelope/Letter idiom.
> > Which just hides the cloning.
> And handles automatically the deletion in containers and some hard
> cases of exception safety.
> I also though the Envelope/Letter idiom would be a good choice for
> symbolic calculus code because type promotion is handled seamlessly.
If you need to support promotion, yes. Envelope/Letter is the way to
go. (That is, after all, what Coplien invented it for.) But for just
simple expressions, where the objects represent nodes in the
expression
tree, it's probably overkill; just using pointers (shared_ptr if you
don't have anything better for memory management) is largely
sufficient,
and considerably easier to implement.
> > If Expression objects are immutable (and they probably should be),
> > then a much simpler solution would be to use reference semantics
> > (i.e. pointers) everywhere. Depending on the application, this
> > might even be a case where shared_ptr is appropriate---an expression
> > tree is just that, a tree, and can't have cycles. (There are other
> > solutions, with better performance, but they generally require more
> > work.)
> Yes. Or something like boost's intrusive_ptr<>; AMA it wouldn't be too
> onerous in development.
Or installing garbage collection. But installation of the Boehm
collector isn't always trivial. But what I was thinking of was some
sort of block allocator, where all of the nodes in a given expression
(and no other nodes) are allocated by a separate instance of the
allocator, and when finished with the entire expression, you simply
destruct the allocator, and that frees all of the memory it manages.
Without the need to walk the tree deleting the nodes one by one.
(Done
correctly, allocating a single node is just a comparison and a
subtraction -- an order of magnitude faster than more general
allocators --, and you never free an individual node.)
--
James Kanze
I believe the point you raise was addressed in the immediately following paragraph:
>> Additionally, the copy is now a Base, so any overriding of
>> functionality in Derived is lost.
> In many cases (and almost certainly in his), the base class should be
> abstract, which guarantees no slicing (since you can't have instances
> of
> an abstract type).
I wouldn't rely on such a guarantee.
More to the point, you don't yourself rely on such a guarantee. :-)
Because nothing stops anyone from deriving a concrete class with further dervied
classes.
I can think of many ways that someone inadvertently declares an automatic
variable of some concrete derived class.
I think it's better to just design that possible bug vector away.
It's the distinction that you often make in this group between objects with
identity and those with just value, where the former are best designed so that
they can only be used with dynamic allocation.
That's a per class solution. And it requires one factory function per
constructor. That's sort of ugly, not to mention laborious, and since you're
designing a common PolymorphicObject base class I think you may save a lot of
work by centralizing the functionality there, -- even though it relies on a
convention, that all derived classes also declare destructors protected (it's a
shame that the accessibility can't be inherited automatically!).
Cheers,
- Alf
Your remark is very interesting, thank you for that.
It's a pitty this has not been done for C++.
Cristian Barbarosie http://cmaf.ptmat.fc.ul.pt/~barbaros/
Well, it seems to me.
Have you looked at it recently ?
I have retried 2 seconds ago and it was still up.
Lest update dates to November 4, 2009.
--
Michael
Boost's Spirit/Phoenix can get you a long way to do the same but it is
not for the faint of heart :)
--
Michael
How about having a pure virtual destructor in the base class? Can you
replace these two points with this idea?
> >> Base o = Derived();
Yes, but I found the emphasis on sizeof misleading. The possibly
different sizes may have something to do with the original
motivation---I don't know, but the fact remains that size is
irrelevant
in the modern language.
> > In many cases (and almost certainly in his), the base class should
> > be abstract, which guarantees no slicing (since you can't have
> > instances of an abstract type).
> I wouldn't rely on such a guarantee.
> More to the point, you don't yourself rely on such a guarantee. :-)
It depends on context.
> Because nothing stops anyone from deriving a concrete class with
> further dervied classes.
Well, there's always common sense:-). At some point, you can't
possibly
protect against everything, and if the user is going to ignore the
basic
design of the class completely, he's hosed, and there's not much you
can do about it. In this particular case, from what has been
presented
here, Expression will in fact be an abstract base class, and there
will
only be one (or possibly two, but the intermediate level will also be
abstract) level of derivation.
In other contexts, of course, the situation might be different.
[...]
> >> * Make sure that objects can only be created dynamically.
> >> The reasonable way is to make the destructor protected.
> > In practice, I suspect that this may be overkill if the base class
> > is abstract. Except for construction, client code will only use the
> > base class. And there's no way they can accidentally declare a
> > variable with the type of the base class.
> I can think of many ways that someone inadvertently declares an
> automatic variable of some concrete derived class.
> I think it's better to just design that possible bug vector away.
> It's the distinction that you often make in this group between objects
> with identity and those with just value, where the former are best
> designed so that they can only be used with dynamic allocation.
Interestingly enough, I've never tried to enforce these rules. And
I've
never had problems with clients trying to allocate entity objects
other
than dynamically---by their very nature, they have an open lifetime
which doesn't coincide with any scope. (I have had cases of people
allocating value objects dynamically when they shouldn't. And I do
sometimes wonder if I shouldn't ban dynamic allocation for these. But
then, I'd have to ban it for double and int as well, and I don't know
how to do that. In the end, in such cases, the only solution is
education.)
Anyhow, my choice here is based on my experiences (i.e. the
programmers
I've worked with). Your experiences may be different, and if I did
find
myself in a context where such errors were occuring, I'd take
appropriate steps to prevent them.
There are IMHO two separate issues. One is declaring instances on the
stack. The other is ensuring that there are no raw pointers to the
object. Essential if you hope to use shared_ptr (which is very
dangerous, and should be avoided in general), and in this case,
possible, since the basic semantics of an expression node are such
that
we can reasonably assume that derived classes will never export their
this pointer. For the second, the only solution is some sort of
factory
function. And again, for the special case of expression nodes, the
factory function can easily be a template (which still requires that
each derived class declare it friend), although I think I'd still
rather
use the boilerplate. (My editor supports copy-paste, and it's not as
if
there's any chance of the code having to be modified.)
--
James Kanze
It doesn't help ensure the use of a smart pointer, and it doesn't
prevent allocation on the stack of a derived class. Since in
practice,
there will be pure virtual functions in the base class anyway, I don't
think it buys you anything.
--
James Kanze
I have taken your code and added a copy constructor to Expression. If
you trace through the prints you will see that it gets called when
push_back is called. Also notice that with this copy constructor,
only the nname information can be copied into the new Expression
objects. I think that in your example, the compiler is creating its
own copy constructor that is doing something similar to the one I
provided.
Feel free to ask if you have any more questions.
#include <iostream>
#include <list>
using namespace std;
class Expression {
public:
string nname;
Expression ()
{ cout << "constructor Expression, no name provided" << endl;}
Expression (string n)
{ cout << "constructor Expression, name " << n << endl;
nname = n; }
virtual string name()
{ cout << "method Expression::name"<< endl; return nname; }
// Copy constructor:
Expression (const Expression &expression)
{ cout << "constructor Expression, expression provided" << endl;
nname = expression.nname;}
};
class ExpressionProduct : public Expression {
public:
list<Expression> factors;
ExpressionProduct ()
{cout << "constructor ExpressionProduct, no arguments" << endl;}
ExpressionProduct (const Expression &f)
{ cout << "constructor ExpressionProduct, one argument" << endl;
factors.push_back(f); }
ExpressionProduct (const Expression &f, const Expression &g)
{ cout << "constructor ExpressionProduct, two arguments" << endl;
factors.push_back(f);
factors.push_back(g); }
string name()
{ cout << "method ExpressionProduct::name"<< endl;
list<Expression>::iterator i=factors.begin();
if (i==factors.end()) return "1";
string n;
for (;i!=factors.end();i++) n = n + i->name();
return n; }
};
class ExpressionSum : public Expression {
public:
list<Expression> terms;
ExpressionSum ()
{cout << "constructor ExpressionSum, no arguments" << endl;}
void add (Expression &m)
{ cout << "adding" << endl; terms.push_back(m); }
string name()
{ cout << "method ExpressionSum::name"<< endl;
list<Expression>::iterator i=terms.begin();
if (i==terms.end()) return "0";
string n=i->name();
i++ ;
for (;i!=terms.end();i++)
n = n + '+' + i->name();
i not understand much but this seems ok
#include <iostream>
#include <list>
#include <string>
using namespace std;
class Expression {
public:
string nname;
Expression()
{cout<< "constructor Expression, no name provided\n";}
Expression(string n)
{cout << "constructor Expression, name " << n << "\n";
nname=n;
}
string name()
{cout << "method Expression::name\n";
return nname;
}
};
class ExpressionProduct:public Expression {
public:
list<Expression> factors;
ExpressionProduct()
{cout << "constructor ExpressionProduct, no arguments\n";}
ExpressionProduct(Expression& f)
{cout << "constructor ExpressionProduct, two arguments\n";
factors.push_back(f);
}
ExpressionProduct(Expression& f, Expression& g)
{cout << "constructor ExpressionProduct, two arguments\n";
factors.push_back(f);
factors.push_back(g);
}
string name()
{list<Expression>::iterator i;
string n="";
cout << "method ExpressionProduct::name >>>>>";
for(i=factors.begin(); i!=factors.end(); ++i)
n+=i->name();
return n;
}
};
class ExpressionSum : public Expression {
public:
list<Expression> terms;
ExpressionSum ()
{cout << "constructor ExpressionSum, no arguments\n";}
void add(Expression m){terms.push_back(m);}
void add(Expression& m, Expression& m1)
{terms.push_back(m); terms.push_back(m1);}
string name()
{list<Expression>::iterator i;
string n="";
cout << "method ExpressionSum::name >>>>>> ";
for(i=terms.begin(); i!=terms.end(); ++i)
{if(n!="") n+='+';
cout << "[" << i->name() << "]";
n+=i->name();
}
return n;
}
};
Expression f(ExpressionProduct& g)
{list<Expression>::iterator i;
static string n;
n="";
for(i=g.factors.begin(); i!=g.factors.end(); ++i)
n+=i->name();
return n;
}
Expression h(ExpressionSum& g)
{list<Expression>::iterator i;
static string n;
n="";
for(i=g.terms.begin(); i!=g.terms.end(); ++i)
{if(n!="") n+='+';
cout << "[" << i->name() << "]";
n+=i->name();
}
return n;
}
int main(void)
{ Expression x("x"), y("y"), z("z");
ExpressionProduct xy(x,y), yz(y,z);
ExpressionSum p;
cout << xy.name() << endl;
p.add( f(xy) );
cout << "pname=" <<p.name() << endl;
p.add( f(yz) );
p.add( z );
cout << p.name() << endl;
}
basically it seems your code call the function p.add() not to
one expression but to one "ExpressionProduct"
so there is the need of some function that go to
"ExpressionProduct" to "Expression"
the same there is a need of one function that goes
from ExpressionSum --> to "Expression"
but i'm not expert so don't know if memory is free ok
constructor Expression, name x
constructor Expression, name y
constructor Expression, name z
constructor Expression, no name provided
constructor ExpressionProduct, two arguments
constructor Expression, no name provided
constructor ExpressionProduct, two arguments
constructor Expression, no name provided
constructor ExpressionSum, no arguments
method ExpressionProduct::name >>>>>method Expression::name
method Expression::name
xy
method Expression::name
method Expression::name
constructor Expression, name xy
method ExpressionSum::name >>>>>> method Expression::name
[xy]method Expression::name
pname=xy
method Expression::name
method Expression::name
constructor Expression, name yz
method ExpressionSum::name >>>>>> method Expression::name
[xy]method Expression::name
method Expression::name
[yz]method Expression::name
method Expression::name
[z]method Expression::name
xy+yz+z
my book says that so Expression C ExpressionProduct
> public:
> list<Expression> factors;
> ExpressionProduct ()
then i have to understand why the class "ExpressionProduct"
has in it one element "Expression" so one element "string nname;"
other than "list<Expression> factors;"
yes all true
there is someone that know how to write
void ExpressionProduct::mul(ExpressionSum& g);
{list<Expression>::iterator i;
static Expression n;
n.nname="";
for(i=g.terms.begin(); i!=g.terms.end(); ++i)
{if(n.nname!="") n.nname+='+';
n.nname+=i->name();
}
factors.push_back(n);
}
------------------------------------------
#include <iostream>
#include <list>
#include <string>
using namespace std;
class Expression {
public:
string nname;
Expression(){}
Expression(string n){nname=n;}
string name(){return nname;}
};
class ExpressionSum;
class ExpressionProduct{
public:
list<Expression> factors;
ExpressionProduct(){}
ExpressionProduct(Expression& f){factors.push_back(f);}
ExpressionProduct(Expression& f, Expression& g)
{factors.push_back(f); factors.push_back(g);}
void mul(Expression& m){factors.push_back(m);}
void mul(Expression& m, Expression& m1)
{factors.push_back(m); factors.push_back(m1);}
void mul(ExpressionSum& g);
string name()
{list<Expression>::iterator i;
string n="";
for(i=factors.begin(); i!=factors.end(); ++i)
n+=i->name();
return n;
}
};
class ExpressionSum{
public:
list<Expression> terms;
ExpressionSum (){}
void add(Expression& m){terms.push_back(m);}
void add(Expression& m, Expression& m1)
{terms.push_back(m); terms.push_back(m1);}
void add(ExpressionProduct& g)
{list<Expression>::iterator i;
static Expression n;
n.nname="";
for(i=g.factors.begin(); i!=g.factors.end(); ++i)
n.nname+=i->name();
terms.push_back(n);
}
string name()
{list<Expression>::iterator i;
string n="";
for(i=terms.begin(); i!=terms.end(); ++i)
{if(n!="") n+='+';
n+=i->name();
}
return n;
}
};
int main(void)
{ Expression x("x"), y("y"), z("z");
ExpressionProduct xy(x,y), yz(y,z);
ExpressionSum p;
cout << xy.name() << endl;
p.add( xy );
cout << "pname=" <<p.name() << endl;
p.add( yz );
p.add( z );
cout << p.name() << endl;
p.add(xy);
cout << "pname2=" <<p.name() << endl;
return 0;
}
----------
xy
pname=xy
xy+yz+z
pname2=xy+yz+z+xy
> i not understand much but this seems ok
No it's not. Not from a design point, anyway, and it doesn't begin to
fulfill the requirements.
What he basically needs is something along the lines of:
// Interface...
class Expression
{
Expression( Expression const& );
Expression& operator=( Expression const& );
public:
virtual ~Expression() {}
virtual double value() const = 0;
// other functions...?
};
class OneOperandExpression : public Expression
{
protected:
OneOperandExpression( Expression const* op )
: m_op( op )
{
}
Expression const* m_op;
public:
virtual ~OneOperandExpression() { delete m_op; }
};
class TwoOperandExpression : public Expression
{
protected:
TwoOperandExpression( Expression const* lhs, Expression const*
rhs )
: m_lhs( lhs )
, m_rhs( rhs )
{
}
Expression* m_lhs;
Expression* m_rhs;
public:
virtual ~TwoOperandExpression() { delete m_lhs; delete
m_rhs; }
};
class ConstantExpression : public Expression
{
double m_value;
public:
ConstantExpression( double value )
: m_value( value )
{
}
virtual double value() const
{
return m_value;
}
};
class NegExpression : public OneOperandExpression
{
public:
NegExpression( Expression const* op )
: OneOperandExpression( op )
{
}
virtual double value() const
{
return - m_op->value();
}
};
class AddExpression : public TwoOperandExpression
{
public:
AddExpression( Expression const* lhs, Expression const* rhs )
: TwoOperandExpression( lhs, rhs )
{
}
virtual double value() const
{
return lhs->value() + rhs->value();
}
};
Systematic use of boost::shared_ptr< Expression const > in place of
Expression const* would simplify things somewhat. Providing static
factory functions for each type, with the actual constructors private,
would ensure that all instances are dynamically allocated (and
immediately put under control of a shared_ptr, if shared_ptr is being
used). Using some sort of "designated" memory management can make
things significantly safer and faster, at the cost of some additional
complexity, including at the client level.
--
James Kanze
so what there is in the class "Expression"
there are only functions or there is something, a pointer, a string type, a
double etc at last some data
Error E2251 str0.cpp 24: Cannot find default constructor to initialize base
class 'Express
ion' in function OneOperandExpression::OneOperandExpression(const Expression *)
Error E2034 str0.cpp 36: Cannot convert 'const Expression *' to 'Expression *'
in function
TwoOperandExpression::TwoOperandExpression(const Expression *,const Expression
*)
Error E2034 str0.cpp 37: Cannot convert 'const Expression *' to 'Expression *'
in function
TwoOperandExpression::TwoOperandExpression(const Expression *,const Expression
*)
Error E2251 str0.cpp 38: Cannot find default constructor to initialize base
class 'Express
ion' in function TwoOperandExpression::TwoOperandExpression(const Expression
*,const Expre
ssion *)
Error E2251 str0.cpp 54: Cannot find default constructor to initialize base
class 'Express
ion' in function ConstantExpression::ConstantExpression(double)
Error E2451 str0.cpp 86: Undefined symbol 'lhs' in function
AddExpression::value() const
Error E2451 str0.cpp 86: Undefined symbol 'rhs' in function
AddExpression::value() const
*** 7 errors in Compile ***
this is what seems at last compile
but i'm sure it not like or i don't like it
#include <iostream.h>
#include <list.h>
#include <string.h>
#include <stdlib.h>
using namespace std;
// Interface...
class Expression{
public:
Expression( ){st="";}
Expression( Expression& m){st=m.st;}
Expression& operator=( Expression& m )
{ st=m.st; return m;}
double f(Expression& m){return atof(m.st.c_str());}
double value(){return f(*this);}
~Expression() {}
string st;
};
class OneOperandExpression
{public:
OneOperandExpression(){m_op=0;}
OneOperandExpression(Expression* op){m_op=op;}
Expression* m_op;
};
class TwoOperandExpression{
public:
TwoOperandExpression(Expression* lhs, Expression* rhs )
{ m_lhs = lhs; m_rhs=rhs;}
Expression* m_lhs;
Expression* m_rhs;
};
class ConstantExpression{
public:
ConstantExpression(double value){ m_value=value;}
virtual double value() {return m_value;}
double m_value;
};
class NegExpression : public OneOperandExpression{
public:
NegExpression(){OneOperandExpression();}
NegExpression(Expression* op){m_op=op;}
double value(){return -(m_op->value());}
};
class AddExpression : public TwoOperandExpression{
public:
AddExpression(Expression* lhs, Expression* rhs )
:TwoOperandExpression( lhs, rhs ){;}
double value()
{return m_lhs->value()+m_rhs->value();}
};
int main(void)
{
return 0;
}
Yes, now I understand. Thank you for your clear explanation.
> >> i not understand much but this seems ok
> > No it's not. Not from a design point, anyway, and it doesn't begin to
> > fulfill the requirements.
> > What he basically needs is something along the lines of:
> > // Interface...
> > class Expression
> > {
> > Expression( Expression const& );
> > Expression& operator=( Expression const& );
> > public:
> > virtual ~Expression() {}
> > virtual double value() const = 0;
> > // other functions...?
> > };
> so what there is in the class "Expression" there are only
> functions or there is something, a pointer, a string type, a
> double etc at last some data
There's nothing. Not even the functions. It's an interface;
you never instantiate it.
> Error E2251 str0.cpp 24: Cannot find default constructor to initialize base
> class 'Express
> ion' in function OneOperandExpression::OneOperandExpression(const Expression *)
I say, "something like". I typed the code in off the top of my
head. I forgot that yes, since you've declared a constructor
(the copy constructor), you no longer get the default
constructor. So you have to provide it as well (protected, and
with an empty body).
> Error E2034 str0.cpp 36: Cannot convert 'const Expression *' to 'Expression *'
> in function
> TwoOperandExpression::TwoOperandExpression(const Expression *,const Expression
> *)
A simple typo. There's a const missing somewhere.
> using namespace std;
Copy construction and assignment were private for a reason. And
not implemented for the same reason.
> double f(Expression& m){return atof(m.st.c_str());}
>
> double value(){return f(*this);}
> ~Expression() {}
> string st;
> };
What is the string there for? Why are any of the constructors
public? Just add an empty default constructor (protected,
although it doesn't really matter).
And the whole point is that value is a pure virtual function
here.
> class OneOperandExpression
> {public:
> OneOperandExpression(){m_op=0;}
> OneOperandExpression(Expression* op){m_op=op;}
Why the assignments? Use initialization (as I initially did).
> Expression* m_op;
And make this a pointer to const. If I can trust your error
messages, that's all that's required.
> };
> class TwoOperandExpression{
> public:
> TwoOperandExpression(Expression* lhs, Expression* rhs )
> { m_lhs = lhs; m_rhs=rhs;}
> Expression* m_lhs;
> Expression* m_rhs;
As above: use initialization, and make the members pointer to
const.
> };
> class ConstantExpression{
> public:
> ConstantExpression(double value){ m_value=value;}
> virtual double value() {return m_value;}
> double m_value;
Here, m_value should be private.
> };
> class NegExpression : public OneOperandExpression{
> public:
> NegExpression(){OneOperandExpression();}
What on earth are you doing here? First, it makes no sense for
NegExpression to have a default constructor, since a negation
operator always has an operand. And what is the purpose of
creating the temporary OneOperandExpression in the constructor
(which wouldn't have been possible with my code, since
OneOperandExpression was an abstract type).
> NegExpression(Expression* op){m_op=op;}
> double value(){return -(m_op->value());}
> };
> class AddExpression : public TwoOperandExpression{
> public:
> AddExpression(Expression* lhs, Expression* rhs )
> :TwoOperandExpression( lhs, rhs ){;}
> double value()
> {return m_lhs->value()+m_rhs->value();}
> };
You seem to put in the virtual keyword more or less at random.
> int main(void)
> {
> return 0;
> }
Somehow, I don't think you've understood what is going on.
--
James Kanze
yes i'm not so smart, but how to use all this?
why declare all distructors "virtual" ?
why use all that "const"?
#include <iostream>
#include <list>
#include <string>
using namespace std;
// Interface...
class Expression{
public:
Expression(){}
Expression( Expression& );
Expression& operator=( Expression& );
virtual ~Expression() {}
virtual double value() = 0;
// other functions...?
};
class OneOperandExpression : public Expression
{public:
Expression* m_op;
OneOperandExpression( Expression* op ):m_op( op ){}
virtual ~OneOperandExpression() { delete m_op; }
};
class TwoOperandExpression : public Expression{
public:
Expression* m_lhs;
Expression* m_rhs;
TwoOperandExpression( Expression* lhs, Expression* rhs )
:m_lhs( lhs ),m_rhs( rhs ){}
virtual ~TwoOperandExpression() { delete m_lhs; delete m_rhs; }
};
class ConstantExpression : public Expression{
public:
double m_value;
ConstantExpression( double value ):m_value( value ){}
virtual double value(){return m_value;}
};
class NegExpression : public OneOperandExpression{
public:
NegExpression( Expression* op ):OneOperandExpression(op){}
virtual double value(){return - m_op->value();}
};
class AddExpression : public TwoOperandExpression{
public:
AddExpression( Expression* lhs, Expression* rhs )
:TwoOperandExpression( lhs, rhs ){}
virtual double value(){return m_lhs->value() + m_rhs->value();}
};
class MulExpression : public TwoOperandExpression{
public:
MulExpression(Expression* lhs, Expression* rhs)
:TwoOperandExpression( lhs, rhs ){}
virtual double value(){return m_lhs->value() * m_rhs->value();}
};
int main(void)
{// Expression x(3), y(4), z(5);
MulExpression xy(3,4), yz(5,9);
AddExpression p;
cout << xy.value() << "\n";
p.add( xy );
cout << "pvalue=" <<p.value() << "\n";
p.add( yz );
p.add( z );
cout << p.value() << "\n";
p.add(xy);
return 0;
}
> > Somehow, I don't think you've understood what is going on.
> yes i'm not so smart, but how to use all this?
It's not a question of "smart", it's a question of what you've
learned (or haven't learned). It this case, given your comments
and your code, I suspect that there's a lot of basic C++ and OO
to which you haven't been exposed. Enough that it can't be
addressed in a simple answer here---a good book is in order. (I
would suggest Stroustrup's most recent book for starters. It's
one of the best tutorial texts for "programming" that I've seen
to date: it doesn't teach C++ as such, it teaches programming,
using C++ as a major tool.)
Until you've actually learned about inheritance, abstract base
classes, and how and why they are used, you won't be able to
make sense out of my code.
> why declare all distructors "virtual" ?
Because once I've constructed an object, I will only use it
through a pointer to the base class. And deleting an object
through a pointer to the base class is only legal if the
destructor is virtual.
> why use all that "const"?
Because I felt like it:-).
Seriously, as designed, I expect all of the derived classes to
be immutable, so const correctness says that all of my functions
should be const, and that all of my pointers be const. In some
cases, const correctness is important, but not when all objects
must be allocated on the heap; I could just as easily dropped
all of the const, with no significant difference. What doesn't
work is mixing the two approaches: either everything const
correct, or nothing.
--
James Kanze
"Good book" defined as?
> (I
>would suggest Stroustrup's most recent book for starters.
The last time I read a Stroustrup's book, it was about the
worst reading I ever had. Hopefully, he learned something
after more than a generation of experience.
But I doubt he can write a book that is brilliant.
When I talked to him at Ruben Engineering in Cambridge Mass.,
what I saw is a conman in front of me. Because I asked him
about the most appropriate question there is, at least at
that time, and that is:
why did you implement C++ as a C preprocessor?
He said no, it is not a C preprocessor, which is a lie,
because it was. And all the objects were patched during
the link phase in the executable, which is totally off
the wall. But fine. We can understand that considering
the state of the art at the moment.
Now, what IS C++?
Well, nothing more than a wrapper for C.
What are the C++ objects?
Well, nothing more than C structures.
Dig?
So, where is that object oriented magic?
Well, in converting the C++ code into C code essentially,
no matter what "expert" is going to blabber what.
> It's
>one of the best tutorial texts for "programming" that I've seen
>to date:
Poor you.
Have you ever read Osho?
Compared to Stroustrup, it is like listening to angels
sing in heaven.
> it doesn't teach C++ as such, it teaches programming,
>using C++ as a major tool.)
C++ is lame. No questions about it.
I do agree with Java chief architect on that one.
Unless you know Java, you'll never be able to understand
how screwed up C++ is.
After working in Java for a few years, I can not even go back
to C++ even though my main program is written in C++.
It just rips my eyes to even look at that code.
So ungly the syntax and the whole trip is.
>Until you've actually learned about inheritance, abstract base
>classes,
Screw those.
What do you need them for?
> and how and why they are used, you won't be able to
>make sense out of my code.
:--}
>> why declare all distructors "virtual" ?
Don't listen to all these "experts".
There are some things you are going to learn.
Some of it does make sense, like things of private/protected/
public level. But most of it is just mental masturbation
that does not buy you anything. Their programs still suck.
>Because once I've constructed an object, I will only use it
>through a pointer to the base class. And deleting an object
>through a pointer to the base class is only legal if the
>destructor is virtual.
>> why use all that "const"?
Because these people keep inventing these things that are
in effect meaningless in the scheme of things.
Yes, they will be telling you: oh, but now we can protect
the const and guarantee they are not going to be modified,
and on and on and on.
But the end result of all these great "improvements" is what?
Well, C++ is history.
What we have now is Python, Ruby, PHP, stinky Javascript, CSS,
HTML and things like that, that constitute a majority of what
is happening in the world right now. Const or template or
generics or you name it is nothing more than a mental
masturbation excersize.
What is happening is portability, dynamic scoping and
interpetation versus compilation.
They can design or "improve" anything they want.
Lil does it matter in the scheme of things.
>Because I felt like it:-).
Well, at least you are honest in saying it is utterly
meaningless.
Anyway, enough for now.
>Seriously, as designed, I expect all of the derived classes to
>be immutable, so const correctness says that all of my functions
>should be const, and that all of my pointers be const. In some
>cases, const correctness is important, but not when all objects
>must be allocated on the heap; I could just as easily dropped
>all of the const, with no significant difference. What doesn't
>work is mixing the two approaches: either everything const
>correct, or nothing.
>
--
Programmer's Goldmine collections:
Tens of thousands of code examples and expert discussions on
C++, MFC, VC, ATL, STL, templates, Java, Python, Javascript,
organized by major topics of language, tools, methods, techniques.
if you pratically
not write something, **one example**, that is compiled
**and run** what you say should be time spend wrong for me
and for you too
in other words it is all "bla bla bla"