Which class should handle this method?

[BillG]

Say I have a class called Invoice. An invoice consists of a header and
detail lines. In my class called Invoice I have a member which is a object
of a collection of DetailLines, defined as follows:

public class Invoice
{
.....
List <DetailLine> detail;
}

When I retrieve an invoice, I also want to retrieve the detail lines. So do
I retrieve the detail lines in my method GetInvoice(long invoiceNo) in the
Invoice class or is it more proper to have an InvoiceController class that
first gets the Invoice and then gets the detail lines and populates the
member field in the Invoice class.

I realize that it works both ways, but what is the best way to do it.

Bill

 

[pieter]

Say I have a class called Invoice. An invoice consists of a header and
detail lines. In my class called Invoice I have a member which is a object
of a collection of DetailLines, defined as follows:

public class Invoice
{
.....
List <DetailLine> detail;

}

When I retrieve an invoice, I also want to retrieve the detail lines. So do
I retrieve the detail lines in my method GetInvoice(long invoiceNo) in the
Invoice class or is it more proper to have an InvoiceController class that
first gets the Invoice and then gets the detail lines and populates the
member field in the Invoice class.

I realize that it works both ways, but what is the best way to do it.

Bill

The best way I personally think would be to encapsulate the fetching
of the detail lines in the invoice class. The invoice class might
delegate the actual fetching. The reason for this as that you might
end up with a "god" class (the controller class). The controller class
becomes a class that in itself has no information, but is purely there
to populate other objects. The temptation later could be to extend the
"god" class to do other work on the Invoice and InvoiceLine classes,
thus removing behaviour from the classes and the classes become pure
data carriers.

Here are a couple of links that might explain better:
http://en.wikipedia.org/wiki/God_object
http://c2.com/cgi/wiki?GodClass
http://www.pbell.com/index.cfm/2006...chitect-Thirteen-Reasons-for-Creating-a-Class

Pieter

 

[Ben Voigt [C++ MVP]]

The best way I personally think would be to encapsulate the fetching
of the detail lines in the invoice class. The invoice class might
delegate the actual fetching. The reason for this as that you might
end up with a "god" class (the controller class). The controller class
becomes a class that in itself has no information, but is purely there
to populate other objects. The temptation later could be to extend the
"god" class to do other work on the Invoice and InvoiceLine classes,
thus removing behaviour from the classes and the classes become pure
data carriers.

Except that now your data access layer and your object model are conflated.
If there's any need to handle different database formats or other storage
and networking protocols in general, then the methods for populating an
Invoice should probably be kept separate from the Invoice class.

 

[pieter]

I agree with your statement. You could pass the data access layer
component into the invoice class, e.g.
GetInvoice(long invoiceNo, IRepository DataAccess) or have a
constructor that takes an implementation of your data access component
as a parameter. If you want to take this scenario even further you
could use an IoC container (e.g. Windsor / MicroKernel from
castleproject.org) to dynamically inject the data access component
into the class.

Thus when the database format (or networking protocols) changes the
Invoice class is not affected as the IRepository implementation takes
care of the fetching, but the invoice class is still responsible for
the population of invoice lines.

The point I was trying to get across is that it normally better to
avoid "god" classes as these classes knows to much about other classes
which results in a tightly coupled system. Again these are just my
humble opinions.

Pieter

 

[Chris Shepherd]

Except that now your data access layer and your object model are conflated.
If there's any need to handle different database formats or other storage
and networking protocols in general, then the methods for populating an
Invoice should probably be kept separate from the Invoice class.

In situations like these I prefer to have a Data Access class specific
to invoices, and have the Invoice class only know that it needs to talk
to an InvoiceDataRetriever (or whatever naming convention suits you) and
that it will be provided via constructor. This way you keep your data
abstraction and also the ability to have the Invoice class manage
retrieving its InvoiceDetail children.

Chris.