Solved

problem with my code

Posted on 2006-12-01
5
210 Views
Last Modified: 2010-04-01

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();
}


0
Comment
Question by:samj
  • 3
5 Comments
 
LVL 4

Accepted Solution

by:
Raymun earned 125 total points
Comment Utility
Hello,

Group group1( combi1 );
Group group2( combi2 );

Have scope only inside names2groups. Once operate() is called, group1 and group2 have already been destroyed. I'm not sure what exactly happens behind the scenes when objects get destroyed but I know it is something along the lines of cleaning up pointers. I suspect that since Group is destroyed, then so will the Consumers it holds, and therefore the pointers to Account.

Your program will work if you combine name2groups() and operate() into one method.

-Ray
0
 

Author Comment

by:samj
Comment Utility
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();
}
0
 
LVL 14

Expert Comment

by:wayside
Comment Utility
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.

0
 

Author Comment

by:samj
Comment Utility
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]$
0
 

Author Comment

by:samj
Comment Utility
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]$
0

Featured Post

Highfive + Dolby Voice = No More Audio Complaints!

Poor audio quality is one of the top reasons people don’t use video conferencing. Get the crispest, clearest audio powered by Dolby Voice in every meeting. Highfive and Dolby Voice deliver the best video conferencing and audio experience for every meeting and every room.

Join & Write a Comment

In days of old, returning something by value from a function in C++ was necessarily avoided because it would, invariably, involve one or even two copies of the object being created and potentially costly calls to a copy-constructor and destructor. A…
Go is an acronym of golang, is a programming language developed Google in 2007. Go is a new language that is mostly in the C family, with significant input from Pascal/Modula/Oberon family. Hence Go arisen as low-level language with fast compilation…
The viewer will be introduced to the technique of using vectors in C++. The video will cover how to define a vector, store values in the vector and retrieve data from the values stored in the vector.
The viewer will be introduced to the member functions push_back and pop_back of the vector class. The video will teach the difference between the two as well as how to use each one along with its functionality.

772 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

10 Experts available now in Live!

Get 1:1 Help Now