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

Just use the this constructor syntax

 public Customer(int ID): this(DataAccess.GetCustomer(ID).Rows[0]) {}

But this construction will throw an exception if you pass it an invalid id (one not in the database.)

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

Answer 4

If you modify your constructors a bit you can get this to work the way you want it... but no, there isn't a way to call another constructor from inside the body of the constructor. But here is what you can do:

Change your constructor that takes DataRows to take a DataTable and call the default constructor first:

Customer( DataTable dt ) : Customer()
{
    if ( dt != null && dt.Rows.Count > 0 )
    {
        // handle the row that was selected
    }
    else
    {
        throw Exception( "customer not in database" ); // or leave this line out to allow a default customer when they arent in the DB
    }
}

Then modify your ID constructor thusly:

Customer(int ID) : Customer(DataAccess.GetCustomer(ID))
{
    // no code
}

So now your default constructor will always be called, and then if the customer was found in the DB you can overwrite the default values with values from the database. If the customer wasn't in the DB you can throw an exception, or just allow the customer to be constructed with only the default values.

Answer 5

If you want this new method to be able to choose whether to construct a Customer from a retrieved data row or to construct an uninitialized Customer and then begin setting its data (e.g. ID) from data at hand, I'd recommend using a factory instead of another constructor. A quick, pseudo-code-like sketch would be:

Customer ProvideCustomer(int ID)
{
    Customer result; // or initialize to null to signal more work to come
    DataTable dt = DataAccess.GetCustomer(ID);
    if (dt.Rows.Count > 0)
    {
        result = new Customer( dt.getappropriaterow ) // however you choose one
    }
    else
    {
        result = new Customer();
        result.ID = ID;          // whatever other initialization you need
    }
    return result;
}

Answer 6

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