CODEWITHSUNDEEP
c#constructor
Mike
Mike

Reputation: 6028

Chain-overloading constructors?

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,

Upvotes: 3

Views: 1971

Answers (5)

Svish
Svish

Reputation: 158301

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

Upvotes: 1

CodingWithSpike
CodingWithSpike

Reputation: 43748

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.

Upvotes: 0

Tanmay
Tanmay

Reputation: 1527

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.

Upvotes: 2

brokenisfixed
brokenisfixed

Reputation: 653

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

Upvotes: 0

Jon Skeet
Jon Skeet

Reputation: 1503349

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).

Upvotes: 12

Related Questions