Bank transaction implementation - side-effects of barriers

I'm interested in implementing role based code in C++. I'm also interested in what side effects or barriers (from static strong typing perhaps?) have been added that I am unaware of and if there is a better way.

```
#include
#include
#include
#include
using namespace std;

// Interfaces to the roles.
class IsourceAccount {
public:
virtual int availableBalance() = 0;
virtual void withdraw(int amount) = 0;
};

class IdestAccount {
public:
virtual int availableBalance() = 0;
virtual void deposit(int amount) = 0;
};

// MODEL Classes.
class Account: public IsourceAccount, public IdestAccount {
private:
int balance;
public:
Account(): balance(0) { }
int availableBalance() { return balance; }
void deposit(int amount) { balance += amount; }
void withdraw(int amount) { balance -= amount; }
};

class Account2: public IsourceAccount, public IdestAccount {
private:
vector audit;
public:
Account2() { }
int availableBalance() { return accumulate( audit.begin(), audit.end(), 0 ); }
void deposit(int amount) { audit.push_back(amount); }
void withdraw(int amount) { audit.push_back(-amount); }
};

// How the Model objects interact
class moneyTransferCTX {
// Role methods (from context and from roleplayers).
class /role/ sourceAccount: public IsourceAccount {
public:
void transferTo(moneyTransferCTX *ctx, int amount);
} *sourceAccount;

class /role/ destAccount: public IdestAccount {
public:
void receive(moneyTransferCTX *ctx, int amount);
} *destAccount;

public:
moneyTransferCTX(IsourceAccount src, IdestAccount dst):
// Cast the objects (src,dst) to their roles (sourceAccount, destAccount).
sourceAccount(static_cast(src)),
destAccount(static_cast(dst))
{ }

void execute(int amount) {
this->sourceAccount->transferTo(this, amount);
}
};

void moneyTransferCTX::sourceAccount::transferTo(moneyTransferCTX *ctx, int amount) {
if (this->availableBalance() withdraw(amount);
ctx->destAccount->

In moneyTransferCTX:

You are using pointers. As you never expect these to be NULL (and they can't be changed) you should be using references.

The constructor is casting objects to things that are not necessarily valid.

moneyTransferCTX(IsourceAccount *src, IdestAccount *dst):
  // Cast the objects (src,dst) to their roles (sourceAccount, destAccount).
  sourceAccount(static_cast(src)),
  destAccount(static_cast(dst))
{}

Just because a src is an IsourceAccount does not mean it is also sourceAccount so casting it may not be valid. But it looks like what you are trying was to create a wrapper class (like this):

class sourceAccount
{
  IsourceAccount&  account;
  public:
    sourceAccount(IsourceAccount &acc): account(acc) {}
    void transferTo(moneyTransferCTX& ctx, int amount)
    {
        if (account.availableBalance() receive(ctx, amount);
    }
}  sourceAccount;

Personally I think this is overkill and you should just put the work you were putting in these classes into the execute method.

I would do this:

// How the Model objects interact
class moneyTransferCTX 
{
  IsourceAccount& sourceAccount;
  IdestAccount&   destAccount;

public:
  moneyTransferCTX(IsourceAccount& src, IdestAccount& dst)
    : sourceAccount(src)
    , destAccount(dst)
  {}

  void execute(int amount)
  {
    if (sourceAccount.availableBalance() < amount) throw -1;
    sourceAccount.withdraw(amount);
    destAccount.deposit(amount);
  }  
};

Creating dynamic objects should not be done if not required.

Account2 *savings = new Account2();
Account *checking = new Account(); //Changing the class type shouldn't hurt the code.

It is easier to create normal objects (then you don't have to delete them). If you must create them then you should wrap their usage in a shared pointer.

Account2 savings;
Account  checking; //Changing the class type shouldn't hurt the code.

Learn to use the C++ streams (It provides type safety):

printf("Opening Status- Savings: %d, Checking: %d\n",
       savings->availableBalance(),
       checking->availableBalance());  

//

std::cout availableBalance()
          availableBalance()
          << "\n";

Your use of exceptions is on the edge of being acceptable. Some people will say it is OK others will say it is in-appropriate. Personally I could go either way depending on what the application is doing. What you should keep in mind that exceptions are used to transfer control when an error occurs but should not normally by used for control flow. Error codes/status can be good internally, but don't leak them outside your interface.

The other thing I would point out is that you should probably not use int (though I suspect you were just doing that as a demonstration) as an exception. Either pick one of the standard exceptions or build your own (derived from std::runtime_error).

try {
    printf("Transfer Savings to Checking 50\n");
    ctx[0]->execute(50);
  } catch(int n) {
    printf("Error: not enough funds\n");
  }

More pointer comments. Normal (modern C++) contains very few pointers. And even less explicit calls to delete. Any dynamically allocated object should be wrapped inside a smart pointer (or container like object boost::ptr_container springs to mind here). This makes there usage exception safe.

std::auto_ptr tr(new moneyTransferCTX(checking, savings));
// Note I am using auto_ptr as this is the only smart pointe in C++03
// But it is not considered a good one (as it is easy to misuse)
// Look up the boost::shared_ptr or if you have C++11 compiler it is part of the standard.

// Note there are several good smart pointers and you should learn when to use
// each one appropriately.

StackExchange Code Review Q#6952, answer score: 3