Link to home
Start Free TrialLog in
Avatar of samj
samj

asked on

problem with my code


can somebody please fix this for me. I am expecting an output like this

jack is withdrawing 5
New balance is 14095
jacky is withdrawing 5
New balance is 14090
dog is withdrawing 5
New balance is 14095
dogy is withdrawing 5
New balance is 14090

but I am getting
jack is withdrawing 5
New balance is 0
jacky is withdrawing 5
New balance is 0
dog is withdrawing 5
New balance is -5
dogy is withdrawing 5
New balance is -5


thanks

#include <string>
#include <iostream>
#include <vector>
using namespace std;

class Account {
  double balance;
public:
  double withdraw( double a) {
    balance -= a;
    return balance;
  }
  Account() : balance( 15000 ) { }
};

//****************************************************************
class Consumer
{
  string name;
  Account* p_acc;
public:
  Consumer(string s, Account* p_a) : name(s), p_acc( p_a ) { }

  void withdraw(int amount){
    cout << name << " is withdrawing " << amount << endl;
    cout << "New balance is " 
       << (*p_acc).withdraw( amount )// <<<<<<<<<<<<<<<< how can I do this?
       << endl;
  }
};

//****************************************************************
class Group
{
  Account acc;
public:
  vector<Consumer> consumers;
  Group(vector<string> cNames) : acc(  )
  {
    group_them( cNames );
  }
  void group_them(vector<string>& r_cNames){
    for(vector<string>::const_iterator i = r_cNames.begin();
      i != r_cNames.end(); i++){
      Consumer consumer( *i, &acc );
      consumers.push_back( consumer );
    }
  }
};

//****************************************************************
class Bank
{
  vector<Group> groups;
  void operate(){
    for( vector<Group>::iterator i_group = groups.begin();
       i_group != groups.end(); i_group++ ){
      for( vector<Consumer>::iterator i_consumer = (*i_group).consumers.begin();
      i_consumer != (*i_group).consumers.end(); i_consumer++ ){
      i_consumer->withdraw( 5 );
      }
    }
  }
  /* some dummy joint accounts for testing */
  void names2groups(){
    vector<string> combi1;
    combi1.push_back( string( "jack" ) );
    combi1.push_back( string( "jacky" ) );
    Group group1( combi1 );

    vector<string> combi2;
    combi2.push_back( string( "dog" ) );
    combi2.push_back( string( "dogy" ) );
    Group group2( combi2 );

    groups.push_back( group1 );
    groups.push_back( group2 );
  }

public:
  Bank()
  {
    names2groups();
    operate();
  }

};


int main(){
  Bank();
}


ASKER CERTIFIED SOLUTION
Avatar of Raymun
Raymun

Link to home
membership
This solution is only available to members.
To access this solution, you must be a member of Experts Exchange.
Start Free Trial
Avatar of samj
samj

ASKER

Ok,
I was toying with boost and found this solution which gave the expected results:
[fred@localhost toy]$ ./proj
jack is withdrawing 5
New balance is 14995
jacky is withdrawing 5
New balance is 14990
dog is withdrawing 5
New balance is 14995
dogy is withdrawing 5
New balance is 14990

#include <string>
#include <iostream>
#include <vector>
#include "boost/smart_ptr.hpp"
using namespace std;

class Account {
  double balance;
public:
  double withdraw( double a) {
    balance -= a;
    return balance;
  }
  Account() : balance( 15000 ) { }
};

//****************************************************************
class Consumer
{
  string name;
  boost::shared_ptr<Account> p_acc;
public:
  Consumer(string s,   boost::shared_ptr<Account> p_a) : name(s), p_acc( p_a ) { }

  void withdraw(int amount){
    cout << name << " is withdrawing " << amount << endl;
    cout << "New balance is " 
       << p_acc->withdraw( amount )// <<<<<<<<<<<<<<<< how can I do this?
       << endl;
  }
};

//****************************************************************
class Group
{
  boost::shared_ptr<Account> p_acc;
public:
  vector<Consumer> consumers;
  Group(vector<string> cNames) : p_acc( new Account )
  {
    group_them( cNames );
  }
  void group_them(vector<string>& r_cNames){
    for(vector<string>::const_iterator i = r_cNames.begin();
      i != r_cNames.end(); i++){
      Consumer consumer( *i, p_acc );
      consumers.push_back( consumer );
    }
  }
};

//****************************************************************
class Bank
{
  vector<Group> groups;
  void operate(){
    for( vector<Group>::iterator i_group = groups.begin();
       i_group != groups.end(); i_group++ ){
      for( vector<Consumer>::iterator i_consumer = (*i_group).consumers.begin();
      i_consumer != (*i_group).consumers.end(); i_consumer++ ){
      i_consumer->withdraw( 5 );
      }
    }
  }
  /* some dummy joint accounts for testing */
  void names2groups(){
    vector<string> combi1;
    combi1.push_back( string( "jack" ) );
    combi1.push_back( string( "jacky" ) );
    Group group1( combi1 );

    vector<string> combi2;
    combi2.push_back( string( "dog" ) );
    combi2.push_back( string( "dogy" ) );
    Group group2( combi2 );

    groups.push_back( group1 );
    groups.push_back( group2 );
  }

public:
  Bank()
  {
    names2groups();
    operate();
  }

};


int main(){
  Bank();
}
You changed more than just using boost, you are now dynamically creating the accounts:

-> Group(vector<string> cNames) : p_acc( new Account )

You still have the scoping issues mentioned by Raymun.

Try this:  add destructors to all your classes, and prints to all your constructors:

class Account {
  double balance;
public:
  double withdraw( double a) {
    balance -= a;
    return balance;
  }
  Account() : balance( 15000 ) { cout << "creating Account" << endl; }
  ~Account() { cout << "deleting Account" << endl; }
};

Your original pre-boost code would print something like

creating Account
creating Group
creating Consumer jack
deleting Consumer jack
deleting Consumer jack
creating Consumer jacky
deleting Consumer jack
deleting Consumer jacky
deleting Consumer jacky
creating Account
creating Group
creating Consumer dog
deleting Consumer dog
deleting Consumer dog
creating Consumer dogy
deleting Consumer dog
deleting Consumer dogy
deleting Consumer dogy
deleting Group
deleting Consumer jack
deleting Consumer jacky
deleting account
deleting Group
deleting Consumer jack
deleting Consumer jacky
deleting account
deleting Group
deleting Consumer dog
deleting Consumer dogy
deleting account
deleting Group
deleting Consumer dog
deleting Consumer dogy
deleting account
deleting Group
deleting Consumer jack
deleting Consumer jacky
deleting account
jack is withdrawing 5
New balance is 14995
jacky is withdrawing 5
New balance is -5
dog is withdrawing 5

and then crash.

You can see this is clearly not what you are expecting.

One solution may be to add a copy constructers and operator= for all your classes, so that when you push them into vectors the objects get properly called. I didn't try this.

Another would be to dynamically create everything and store vectors of pointers, then clean up in Bank's destructor (which you don't have). I did try this, it works fine.

Also, this:

int main(){
  Bank();
}

Is not very good stylistically or architecturally. Someone who has to read or maintain your code would have to spend a lot of time, or use a debugger, to figure out what it did.

Avatar of samj

ASKER

the code below is the same "boost" code I posted but with constructor/destructors with the "cout statment", the oupout is below the code, I will alse try to look into (combine name2groups() and operate() into one method) and report the findings.

#include <string>
#include <iostream>
#include <vector>
#include "boost/smart_ptr.hpp"
using namespace std;

class Account {
  double balance;
public:
  double withdraw( double a) {
    balance -= a;
    return balance;
  }
  Account() : balance( 15000 ) { cout << "Account created." << endl; }
  ~Account(){ cout << "Account distroyed." << endl; }
};

//****************************************************************
class Consumer
{
  string name;
  boost::shared_ptr<Account> p_acc;
public:
  Consumer(string s, boost::shared_ptr<Account> p_a) :
    name(s),
    p_acc( p_a )
  { cout << "Consumer created." << endl; }
  ~Consumer() { cout << "Consumer distroyed." << endl; }

  void withdraw(int amount){
    cout << name << " is withdrawing " << amount << endl;
    cout << "New balance is " 
       << p_acc->withdraw( amount )// <<<<<<<<<<<<<<<< how can I do this?
       << endl;
  }
};

//****************************************************************
class Group
{
  boost::shared_ptr<Account> p_acc;
public:
  vector<Consumer> consumers;
  Group(vector<string> cNames) : p_acc( new Account )
  {
    cout << "Group created." << endl;
    group_them( cNames );
  }
  ~Group() { cout << "Group distroyed." << endl; }

  void group_them(vector<string>& r_cNames){
    for(vector<string>::const_iterator i = r_cNames.begin();
      i != r_cNames.end(); i++){
      Consumer consumer( *i, p_acc );
      consumers.push_back( consumer );
    }
  }
};

//****************************************************************
class Bank
{
  vector<Group> groups;
  void operate(){
    for( vector<Group>::iterator i_group = groups.begin();
       i_group != groups.end(); i_group++ ){
      for( vector<Consumer>::iterator i_consumer = (*i_group).consumers.begin();
      i_consumer != (*i_group).consumers.end(); i_consumer++ ){
      i_consumer->withdraw( 5 );
      }
    }
  }
  /* some dummy joint accounts for testing */
  void names2groups(){
    vector<string> combi1;
    combi1.push_back( string( "jack" ) );
    combi1.push_back( string( "jacky" ) );
    Group group1( combi1 );

    vector<string> combi2;
    combi2.push_back( string( "dog" ) );
    combi2.push_back( string( "dogy" ) );
    Group group2( combi2 );

    groups.push_back( group1 );
    groups.push_back( group2 );
  }

public:
  Bank()
  {
    cout << "Bank created." << endl;
    names2groups();
    operate();
  }
  ~Bank(){ cout << "Bank distroyed." << endl; }
};

int main(){
  Bank();
}

//*********** output *************
[fred@localhost toy]$ ./proj
Bank created.
Account created.
Group created.
Consumer created.
Consumer distroyed.
Consumer created.
Consumer distroyed.
Consumer distroyed.
Account created.
Group created.
Consumer created.
Consumer distroyed.
Consumer created.
Consumer distroyed.
Consumer distroyed.
Group distroyed.
Consumer distroyed.
Consumer distroyed.
Group distroyed.
Consumer distroyed.
Consumer distroyed.
Group distroyed.
Consumer distroyed.
Consumer distroyed.
jack is withdrawing 5
New balance is 14995
jacky is withdrawing 5
New balance is 14990
dog is withdrawing 5
New balance is 14995
dogy is withdrawing 5
New balance is 14990
Bank distroyed.
Group distroyed.
Consumer distroyed.
Consumer distroyed.
Account distroyed.
Group distroyed.
Consumer distroyed.
Consumer distroyed.
Account distroyed.
[fred@localhost toy]$
Avatar of samj

ASKER

here is the code with Raymun suggestion (combine name2groups() and operate() into one method)
which worked,  the differences are the boost version did it in less steps, and by freeing momory sooner than the other fix.

which version is safer to use?

#include <string>
#include <iostream>
#include <vector>
using namespace std;

class Account {
  double balance;
public:
  double withdraw( double a) {
    balance -= a;
    return balance;
  }
  Account() : balance( 15000 ) {cout << "Account created." << endl; }
  ~Account(){ cout << "Account distroyed." << endl; }
};

//****************************************************************
class Consumer
{
  string name;
  Account* p_acc;
public:
  Consumer(string s, Account* p_a) : name(s), p_acc( p_a ) {cout << "Consumer created." << endl; }
  ~Consumer() { cout << "Consumer distroyed." << endl; }
  void withdraw(int amount){
    cout << name << " is withdrawing " << amount << endl;
    cout << "New balance is "
      << (*p_acc).withdraw( amount )// <<<<<<<<<<<<<<<< how can I do this?
      << endl;
  }
};

//****************************************************************
class Group
{
  Account acc;
public:
  vector<Consumer> consumers;
  Group(vector<string> cNames) : acc(  )
  {
    cout << "Group created." << endl;
    group_them( cNames );
  }
  ~Group() { cout << "Group distroyed." << endl; }
  void group_them(vector<string>& r_cNames){
    for(vector<string>::const_iterator i = r_cNames.begin();
     i != r_cNames.end(); i++){
      Consumer consumer( *i, &acc );
      consumers.push_back( consumer );
    }
  }
};

//****************************************************************
class Bank
{
  vector<Group> groups;
  void groupAndOperate(){
    vector<string> combi1;
    combi1.push_back( string( "jack" ) );
    combi1.push_back( string( "jacky" ) );
    Group group1( combi1 );

    vector<string> combi2;
    combi2.push_back( string( "dog" ) );
    combi2.push_back( string( "dogy" ) );
    Group group2( combi2 );

    groups.push_back( group1 );
    groups.push_back( group2 );

    for( vector<Group>::iterator i_group = groups.begin();
      i_group != groups.end(); i_group++ ){
      for( vector<Consumer>::iterator i_consumer = (*i_group).consumers.begin();
      i_consumer != (*i_group).consumers.end(); i_consumer++ ){
     i_consumer->withdraw( 5 );
      }
    }
  }

public:
  Bank()
  {
    cout << "Bank created." << endl;
    groupAndOperate();
  }
  ~Bank(){ cout << "Bank distroyed." << endl; }
};


int main(){
  Bank();
}

//*********** output *************
[fred@localhost toy]$ ./proj
Bank created.
Account created.
Group created.
Consumer created.
Consumer distroyed.
Consumer created.
Consumer distroyed.
Consumer distroyed.
Account created.
Group created.
Consumer created.
Consumer distroyed.
Consumer created.
Consumer distroyed.
Consumer distroyed.
Group distroyed.
Consumer distroyed.
Consumer distroyed.
Account distroyed.
jack is withdrawing 5
New balance is 14995
jacky is withdrawing 5
New balance is 14990
dog is withdrawing 5
New balance is 14995
dogy is withdrawing 5
New balance is 14990
Group distroyed.
Consumer distroyed.
Consumer distroyed.
Account distroyed.
Group distroyed.
Consumer distroyed.
Consumer distroyed.
Account distroyed.
Bank distroyed.
Group distroyed.
Consumer distroyed.
Consumer distroyed.
Account distroyed.
Group distroyed.
Consumer distroyed.
Consumer distroyed.
Account distroyed.
[fred@localhost toy]$