x
Solved

# problem with my code

Posted on 2006-12-01
Medium Priority
225 Views

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
Question by:samj
• 3

LVL 4

Accepted Solution

Raymun earned 375 total points
ID: 18057014
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

ID: 18057020
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

ID: 18057122
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.

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

ID: 18058094
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

ID: 18058139
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

Question has a verified solution.

Are you are experiencing a similar issue? Get a personalized answer when you ask a related question.

Have a better answer? Share it in a comment.