tags:

views:

360

answers:

6

I'm trying to build a class which will initalise its self either by passing in a reference to a record in a database (with the intention that a query will be run against the database and the returned values for the objects properties will be set therein), or by specifying the values "by hand" - this no database call is required.

I've looked at a couple textbooks to discover the "Best-practice" for this functionality and sadly I've come up short.

I've written a couple sample console apps reflecting what I believe to be the two most probable solutions, but I've no Idea which is the best to implement properly?

Sample App #1 uses what most books tell me is the "preferred" way but most examples given alongside those claims do not really fit the context of my situation. I'm worried in here that the flow is not as readable as App #2.

using System;

namespace TestApp
{
    public class Program
    {
        public static void Main(string[] args)
        {
            var one = new OverloadedClassTester();
            var two = new OverloadedClassTester(42);
            var three = new OverloadedClassTester(69, "Mike", 24);

            Console.WriteLine("{0}{1}{2}", one, two, three);

            Console.ReadKey();
        }        
    }

    public class OverloadedClassTester
    {
        public int ID { get; set; }
        public string Name { get; set; }
        public int age { get; set; }

        public OverloadedClassTester() : this(0) { }

        public OverloadedClassTester (int _ID) : this (_ID, "", 0)
        {
            this.age = 14;  // Pretend that this was sourced from a database
            this.Name = "Steve"; // Pretend that this was sourced from a database
        }

        public OverloadedClassTester(int _ID, string _Name, int _age)
        {
            this.ID = _ID;
            this.Name = _Name;
            this.age = _age;
        }

        public override string ToString()
        {
            return String.Format("ID: {0}\nName: {1}\nAge: {2}\n\n", this.ID, this.Name, this.age);
        }
    }
}

This Sample (App #2) "appears" more readable - in that I think it's easier to see the flow of operation. However it does appear efficient in terms of characters saved :p. Also, is it not dangerous that it calls a method of the object before it's fully initialised, or is this not a concern?

using System;

namespace TestApp
{
    public class Program
    {
        public static void Main(string[] args)
        {
            var one = new OverloadedClassTester();
            var two = new OverloadedClassTester(42);
            var three = new OverloadedClassTester(69, "Mike", 24);

            Console.WriteLine("{0}{1}{2}", one, two, three);

            Console.ReadKey();
        }        
    }

    public class OverloadedClassTester
    {
        public int ID { get; set; }
        public string Name { get; set; }
        public int age { get; set; }

        public OverloadedClassTester()
        {
            initialise(0, "", 21); // use defaults.
        }

        public OverloadedClassTester (int _ID)
        {
            var age = 14;  // Pretend that this was sourced from a database (from _ID)
            var Name = "Steve"; // Pretend that this was sourced from a database (from _ID)

            initialise(_ID, Name, age);
        }

        public OverloadedClassTester(int _ID, string _Name, int _age)
        {
            initialise(_ID, _Name, _age);
        }

        public void initialise(int _ID, string _Name, int _age)
        {
            this.ID = _ID;
            this.Name = _Name;
            this.age = _age;
        }

        public override string ToString()
        {
            return String.Format("ID: {0}\nName: {1}\nAge: {2}\n\n", this.ID, this.Name, this.age);
        }
    }
}

What is the "correct" way to solve this problem, and why?

Thanks,

+9  A: 

I would definitely chain the constructors, so that only one of them does the "real work". That means you only have to do the real work in one place, so if that work changes (e.g. you need to call some validation method at the end of the constructor) you only have one place where you need to change the code.

Making "simple" overloads call overloads with more parameters is a pretty common pattern IME. I find it more readable than the second version, because you can easily tell that calling one overload is going to be the same as calling the "bigger" one using the default values. With the second version, you have to compare the bodies of the constructors.

I try not to have more than one constructor which chains directly to the base class wherever possible - unless it's chaining to a different base class constructor, of course (as is typical with exceptions).

Jon Skeet
Personally I do prefer solution #1. My primary concern stems from the fact that variable assignment is happening retrospectively to the call to the "biggest" constructor. This appeared a little disjointed to me at first, but I suppose I can put this down to the learning curve? :)
Mike
I'm not sure I quite understand what you mean in the second sentence... if you could elaborate, I'll explain :)
Jon Skeet
Mike
Yes, that's what will happen - but it would be better to provide the "14" and "Steve" to the original constructor. Overwriting the values after calling the chained constructor isn't generally a good idea - ideally, all but the most complex constructor should *just* call the complex one.
Jon Skeet
A: 

Hello,

maybe it would be better to use optional parameters? In this case, you would create a single constructor and initialize the values to the parameters you wish to set.

More information: link text

fixed
Perhaps when version 4.0 of the language is released?
Daniel Earwicker
Yes, I've been playing arround with the 4.0 .NET - it is really nice.
fixed
+2  A: 

Do not use database calls in a constructor. This means your constructor is doing a lot of work. See http://misko.hevery.com/code-reviewers-guide/ (Google guide for writing testable code).

Apart from this, chaining constructors (option 2) looks good. Mostly because as you say it is readable. But why are you assigning this.Name etc in the constructor and doing it again in initialize. You could assign all values in initialize.

Tanmay
A: 

I prefer #1, the chaining constructors, from a maintenance perspective. When someone comes back to edit the code later on, you wouldn't want them to edit the initialise() method and not realize that it is being called from a constructor. I think it is more intuitive to have all the functional parts in a constructor of some kind.

Personally, I use constructor chaining like that all the time.

rally25rs
+1  A: 

Maybe something like this?

public class OverloadedClassTester
{
    public int Id { get; private set; }
    public string Name { get; private set; }
    public int Age { get; private set; }


    public OverloadedClassTester (Person person)
      : this (person.Id, person.Name, person.Age) { }

    public OverloadedClassTester(int id, string name, int age)
    {
        Id = id;
        Name = name;
        Age = age;
    }

    public override string ToString()
    {
        return String.Format("Id: {0}\nName: {1}\nAge: {2}\n\n", 
            Id, Name, Age);
    }
}
Svish
A: 

This is a case where the builder pattern would really help. There is a modified version defined by Joshua Block (sorry it's java, but the solution should be easy to adapt). Basically define a single constructor that has all of the parameters you want. Then define a static inner class that records the properites you want to put into the object. For instance:

   pubic class Foo {
     public Foo(Param a, Param b, Param c) {
        // assign values
     }

     public class Builder {
        Param a,b,c;
        public void setA(Param a) { this.a = a;}
        public void setB(Param b) { this.b = b;}
        public void setC(Param c) { this.c = c;}
        public Foo build() {
             a = (a==null)?"bob":a;
             b = (b==null)?"sue":b;
             c = (c==null)?"mort":c;
             return new Foo(a,b,c);
        }
     } 

   }

The advantage is that you can build the object with a lot more flexibility, and maintain a simple to read constructor. The builder is responsible for pushing appropriate variables in, and can do DB connection, validation and whatnot before the object is built. You also get nice named constructor parameters.

To instantiate this object:

Foo.Builder builder = new Foo.Builder()
           .setA("blarb")
           .setB("blarb2");
Foo foo = builder.build();

Foo for thought.

reccles