Why down-casting is done here?

[mead]

The code is from a Meyers' book...

class BankAccount { ... }; // as above

// new class representing accounts that bear interest
class InterestBearingAccount: public BankAccount {
public:
virtual void creditInterest() = 0;

...

};

class SavingsAccount: public InterestBearingAccount {

... // as above

};

class CheckingAccount: public InterestBearingAccount {

... // as above

};

// better, but still not perfect
for (list<BankAccount*>::iterator p = allAccounts.begin();
p != allAccounts.end();
++p) {

static_cast<InterestBearingAccount*>(*p)->creditInterest();

}

Why did he use downcasting here to convert BankAccount* to
InterestBearingAccount*? Seems to this isn't necessary since
creditInterest() is a virtual function and creditInterest() in
InterestBearingAccount will be virtual and also in SavingsAccount and
CheckingAccount. So, (*p)->creditInterest(); will invoke creditInterest() in
SavingAccount or CheckingAccount as what virtual function supposes to work
like. Do I miss something? Thanks!

 

[Denis Remezov]

True, and a very good response, but the use of static_cast does ring bells.
Sure it is OK if you've enforced that every object in allAccounts is an
InterestBearingAccount, but in that case we should probably have used
list<InterestBearingAccount*>. I think it's safer to use dynamic_cast. You
have to compare the result of dynamic_cast to NULL, and only if it is not
NULL call the creditInterest virtual function. We could also add a virtual
function creditInterest to the base class with the default behavior to do
nothing. Not sure which is the better solution though.

To me, a dynamic_cast rings other bells (as it very often does). It might be
a sign we are putting more logic into this particular loop than we should.
If creditInterest() doesn't belong to the base class then I would prefer

for_each(allInterestBearingAccounts.begin(),
allInterestBearingAccounts.end(),
mem_fun(&InterestBearingAccoun::creditInterest));

where allInterestBearingAccounts might be, as you said, a
list<InterestBearingAccount*>.
It might as well be a result of a query (in /whatever/ form) for interest
bearing accounts based on allAccounts.

Denis

 

[Leor Zolman]

The code is from a Meyers' book...

class BankAccount { ... }; // as above

Note that, in BankAccount (Item 39 in Effective C++), there's no
creditInterest() member function.

[snip]

// better, but still not perfect
for (list<BankAccount*>::iterator p = allAccounts.begin();
p != allAccounts.end();
++p) {

static_cast<InterestBearingAccount*>(*p)->creditInterest();

}

Why did he use downcasting here to convert BankAccount* to
InterestBearingAccount*? Seems to this isn't necessary since
creditInterest() is a virtual function and creditInterest() in
InterestBearingAccount will be virtual and also in SavingsAccount and
CheckingAccount.

Yes, but the iterator refers to a BankAccount *, and as mentioned above,
those have no awareness of anything by the name of "creditInterest".

So, (*p)->creditInterest(); will invoke creditInterest() in
SavingAccount or CheckingAccount as what virtual function supposes to work
like.

At compile time, calls to member functions through pointers are checked for
legality according to the /static/ type of the pointer. IOW, at compile
time, the compiler can't know what is being pointed to. So it only goes by
what the class (BankAccount, in this case) provides.

Dynamic binding happens at run-time, but C++ requires the static check at
compile-time before the code can get to run-time...

 

[Siemel Naran]

Yes, but the iterator refers to a BankAccount *, and as mentioned above,
those have no awareness of anything by the name of "creditInterest".

True, and a very good response, but the use of static_cast does ring bells.
Sure it is OK if you've enforced that every object in allAccounts is an
InterestBearingAccount, but in that case we should probably have used
list<InterestBearingAccount*>. I think it's safer to use dynamic_cast. You
have to compare the result of dynamic_cast to NULL, and only if it is not
NULL call the creditInterest virtual function. We could also add a virtual
function creditInterest to the base class with the default behavior to do
nothing. Not sure which is the better solution though.

 

[Denis Remezov]

[snip]

I think dynamic_cast has its place in good programming, although we should
take care to limit its use. Suppose you had a list of allAccounts where the
accounts may be interest or non-interest
bearing accounts. Now you want to construct a list of interest-bearing
accounts, for use in your code snippet above. You still have to use
dynamic_cast.

I certainly agree in general, but I feel that by redesigning the base
BankAccount you could find a very sensible alternative or two in this specific
case.

[snip]

BTW, the copy_if function above copies elements from the first container to
the new that meet a certain condition. But I don't think there is such a
function in the standard. Does anyone know of a similar function in the
standard? I think there's got to be one.

Would be nice to have it, but I guess we cannot have everything.

Denis

 

[Siemel Naran]

Siemel Naran wrote:

To me, a dynamic_cast rings other bells (as it very often does). It might be
a sign we are putting more logic into this particular loop than we should.
If creditInterest() doesn't belong to the base class then I would prefer

for_each(allInterestBearingAccounts.begin(),
allInterestBearingAccounts.end(),
mem_fun(&InterestBearingAccoun::creditInterest));

where allInterestBearingAccounts might be, as you said, a
list<InterestBearingAccount*>.
It might as well be a result of a query (in /whatever/ form) for interest
bearing accounts based on allAccounts.

I think dynamic_cast has its place in good programming, although we should
take care to limit its use. Suppose you had a list of allAccounts where the
accounts may be interest or non-interest
bearing accounts. Now you want to construct a list of interest-bearing
accounts, for use in your code snippet above. You still have to use
dynamic_cast.

copy_if(allAccounts.begin(),
allAccounts.end(),
std::back_inserter(allInterestBearingAccounts),
isA<InterestBearingAccount>());

where

template <class T>
struct isA {
template <class U>
bool operator()(const U * u) const {
return dynamic_cast<const T *>(u);
}
};

BTW, the copy_if function above copies elements from the first container to
the new that meet a certain condition. But I don't think there is such a
function in the standard. Does anyone know of a similar function in the
standard? I think there's got to be one.

 

[Siemel Naran]

Would be nice to have it, but I guess we cannot have everything.

replace_copy_if seems pretty close, but not exactly what I'm looking for,
because the destination container will always have the same size as the
source container.

 

[Leor Zolman]

copy_if(allAccounts.begin(),
allAccounts.end(),
std::back_inserter(allInterestBearingAccounts),
isA<InterestBearingAccount>());

where

template <class T>
struct isA {
template <class U>
bool operator()(const U * u) const {
return dynamic_cast<const T *>(u);
}
};

BTW, the copy_if function above copies elements from the first container to
the new that meet a certain condition. But I don't think there is such a
function in the standard. Does anyone know of a similar function in the
standard? I think there's got to be one.

This one would seem to have fallen through the cracks as far as the present
standard is concerned. Here's a work-around: use remove_copy_if with a
logically reversed predicate. Example:

#include <iostream>
#include <vector>
#include <algorithm>
#include <functional>
using namespace std;

bool b20and30(int n) // predicate: true if n is between 20 and 30 (incl.)
{
return n >= 20 && n <= 30;
}

void showvec(const vector<int> &v)
{
for (int i = 0; i < v.size(); ++i)
cout << v << " ";
cout << endl << endl;
}


int main()
{
vector<int> v;
v.push_back(10);
v.push_back(20);
v.push_back(30);
v.push_back(40);

cout << "Original:" << endl;
showvec(v);

vector<int> result;
remove_copy_if(v.begin(), v.end(),
back_inserter(result),
not1(ptr_fun(b20and30)));

cout << "Result:" << endl;
showvec(result);

return 0;
}

Output, showing "b20and30" applied to a vector "as if" it were a copy_if
type of function:

Original:
10 20 30 40

Result:
20 30


-leor