overloading constructors and reusing code

Question

Let's say I've got an object Customer with a couple properties (ID, FirstName, LastName). I've got the default constructor Customer(), but then I've also got a Customer(DataRow dr), since I load this object from a database and that's a simple way to do it.

I frequently come to a point where I want to set up another constructor, Customer(int ID), for times when I want to load a Customer but I haven't made the trip to the database yet. The simplest way to me seems to be like so:

Customer(int ID)
{
    DataTable dt = DataAccess.GetCustomer(ID);
    if (dt.Rows.Count > 0)
    {
        // pass control to the DataRow constructor at this point?
    }
    else
    {
        // pass control to the default constructor at this point?
    }   
}

It makes sense to reuse the code that's already in the DataRow constructor, but I can't figure out a way to call that and return what it gives me. Through Googling, I've found information about constructor overloading with the : this() syntax, but all those examples seem backwards or incompatible with what I'm trying to do.

So there's a gap in my understanding of constructors, but I can't seem to sort it out. What am I missing?

Answer 1

Simplest solution seems to be: construct another function that does the job you want to do and have both constructors call that function.

Answer 2

I'm concerned that what you do not get is not about constructors, but about Single Responsibility Principle and Loose Coupling.

For instance, the code that you show above means:

• your domain model contains data access code
• you are not reusing any code from a base class which may be injected with the data-access logic in the first place
• your domain object is aware of data structures other than itself or its members, like DataTable or DataRow, which ties it with those data structures and makes it cumbersome to use other data structures.

Of course this assumes that you are not using the ActiveRecord Model, which appears to be the case here, but will still be implemented with tight coupling.

My preference is that a domain object will only contain logic associated to holding and manipulating genuine Customer data, nothing else. As such my constructor for it will be:

class Customer
{
    public Customer(int id, string firstName, string LastName)
    {
        Id = id;
        FirstName = firstName;
        LastName = lastName;
    }

    public int Id { get; set; }
    public string FirstName { get; set; }
    public string LastName { get; set; }
}

UPDATE: That being said, this is a major reason why some people prefer ORMs which allow for POCOs, like NHibernate: there is no need to put data loading logic there.

If this were done in NHibernate, for example, you would've needed a DomainObject base class:

public class Customer : DomainObject

Which in turn can be consumed by an implementation of NHibernate's IRepository:

public class Repository<T> : IRepository where T : DomainObject

This Repository object would contain all the code required for CRUD operations.

If you wish to stick with ADO.NET, one possible solution is to create DAL manager objects for all the loading:

public class CustomerManager
{
    public IList<Customer> LoadCustomers()
    {
        //load all customers here
        foreach (DataRow dr in dt.Table[0])
        {
             yield return new Customer((int) dr["Id"], dr["FirstName"].ToString(), dr["LastName"].ToString());
        }
    }

    public Customer LoadCustomerByID(int id)
    {
        //load one customer here
        return new Customer((int) dr["Id"], dr["FirstName"].ToString(), dr["LastName"].ToString());
    }
}

Of course there are a lot more opportunities to even further promote code reuse here.

Answer 3

Having a number of different constructors that all do totally different things based on the arguments often makes for difficult to read code. A better way to do this is to create a number of static creation methods with intent revealing names on your classes. Then you only have one constructor. You can even make all the constructors private if you want. Clients use the static methods to create instances of your class.

So instead of:

Customer c = new Customer(13, "George", "Bush");
Customer c2 = new Customer(12);
Customer c3 = new Customer(GetDataRow(11));

You get:

Customer c = new Customer(13, "George", "Bush");
Customer c2 = Customer.LoadFromDatabaseId(12);
Customer c3 = Customer.MapFromDataRow(GetDataRow(11));

Your customer class then looks like this:

class Customer
{
    public Customer(int id, string firstName, string lastName)
    {
        //...
    }

    static public Customer MapFromDataRow(DataRow dr)
    {
        return new Customer(
            dr["ID"],
            dr["FirstName"],
            dr["LastName"]);
    }

    static public Customer LoadFromDatabaseId(int id)
    {
        DataTable dt = DataAccess.GetCustomer(ID);
        if (dt.Rows.Count > 0)    
        {
            return MapFromDataRow(dt.Rows[0]);
        }
        else    
        {        
            throw new CustomerNotFoundException(id);                
        } 
    }
}