Object composition - cannot access object properties

Question

Hi i have a problem with object composition. The class CInvoice needs to have a CCustomer object inside, so I created a constructor that requires a customer.

In Invoice.h file there is a line:

CCustomer *customer;

And the mentioned constructor looks like this:

CInvoice::CInvoice(CCustomer Customer)
{
   customer = &Customer;
}

When I try to print the name of the customer on the invoice it returns some random characters

CCustomer customer("McDonalds", "Boston, Massachusetts", 4);
CInvoice invoice(customer); 

cout << "Customer:" << customer.GetName() << endl; //it prints "McDonalds"
cout << "Invoice.Customer:" << invoice.customer->GetName() << endl; // it prints random characters

Have I implemented the object composition properly?

Also I have a class CInvoiceElement and have a question about it. Should I create invoice elements without having the invoice object created or the other way around? Which is more logical?

Why is the CCustomer* customer a publicly accessible attribute of the invoice? — tmaric, Jan 08 '13 at 14:09
I will set it private later. Im not an experienced programmer and I usually start off with public members and sucessively set them private if needed. Btw the program stores the customers and invoices in the SQL-Server database. Do you think I should pass in the CCustomer* or the customer ID to the CInvoice constructor? — BuahahaXD, Jan 08 '13 at 15:00

score 4 · Answer 1 · answered Jan 08 '13 at 14:08

4

You should pass in the pointer to the CCustomer in the constructor, otherwise you take the address of the copy of CCustomer that is used as argument to the constructor.

Here is what the code should look like:

CInvoice::CInvoice(CCustomer* _customer)
{
   customer = _customer;
}


.... 
CCustomer customer("McDonalds", "Boston, Massachusetts", 4);
CInvoice invoice(&customer); 

cout << "Customer:" << customer.GetName() << endl; //it prints "McDonalds"
cout << "Invoice.Customer:" << invoice.customer->GetName() << endl; // it prints random characters

answered Jan 08 '13 at 14:08

Ivaylo Strandjev

69,226
18
123
176

The code example provided is only valid as long as the lifetime of both the `customer` and `invoice` objects are the same. If the lifetime of the `invoice` object is longer than the `customer` object, the problem still exists. – Some programmer dude Jan 08 '13 at 14:11
@JoachimPileborg I know I just wanted to make it easier to understand what I meant by my answer. I am not saying this is the best way to solve the problem, simply indicating the problem with OP's code. – Ivaylo Strandjev Jan 08 '13 at 14:13

Jack · Accepted Answer · 2013-01-08T14:17:35.527

4

CInvoice::CInvoice(Customer customer)
{
   customer = &Customer;
}

When you call this method what happens is that:

you call CInvoice(customer)
a copy of customer is pushed onto the stack as the argument
the address of the copy is assigned to Customer *customer;
the constructor ends
stack is released and the customer argument becomes an invalid pointer
Customer *customer thus points to garbage

What you should do is to allocate Customer on the heap and pass a pointer, eg.

Customer *customer = new Customer();
CInvoice *invoice = new CInvoice(customer);

CInvoice::CInvoice(Customer *customer) {
  this->customer = customer;
}

In this way your customer instance is allocated in the heap and it persists the scope in which you declare it. The example given by izomorphius works as well but Customer is local to the scope (it's automatically allocated onto stack), once you exit from the scope of the function the pointer inside CInvoice becomes invalid. I hope you get the difference.

edited Jan 08 '13 at 14:17

answered Jan 08 '13 at 14:10

Jack

131,802
30
241
343

I never used the new keyword in C++ (I use it all the time in C#). Should it be used to assigning to the pointers? Is it fine to create objects like this: CCustomer customer; ? – BuahahaXD Jan 08 '13 at 15:04
It's different, when you declare a variable as `CCustomer customer` then it is not a pointer do a `CCustomer` instance but something that is allocated in automatically on the stack. In C# (never used is), if it's like Java, you don't even have objects allocated on stack but they are always reference (pointers) to objects in any case. – Jack Jan 08 '13 at 16:01
You are flipping my C++ knowledge upside down. I always used to create objects like this: CClassName objectname;. Now you are telling me to do it this way: CClassName* objectname = new CClassName(); Do I understand it right? – BuahahaXD Jan 08 '13 at 22:06
Both are valid and senseful according to the context. This topic is quite wide and discussing it on comments is quite complex. We can continue on chat if you need some clues but the best thing is to read the specific differences between dynamic heap allocation and automatic stack allocation. – Jack Jan 09 '13 at 00:19

qPCR4vir · Answer 3 · 2013-01-08T14:30:00.290

0

this will not change the rest of your code..

CInvoice::CInvoice(CCustomer &Customer)
{
   customer = &Customer;
}

but maybe you ned?

In Invoice.h file there is a line:

CCustomer customer;

And the mentioned constructor looks like this??:

CInvoice::CInvoice(const CCustomer &Customer)
: customer(Customer)
{
}

edited Jan 08 '13 at 14:30

answered Jan 08 '13 at 14:17

qPCR4vir

3,521
1
22
32

Object composition - cannot access object properties

3 Answers3