56

Maybe the question I've stated isn't the right question, cause I already know the short answer is "you can't".

The situation

I have a base class with an overloaded constructor that takes two arguments.

class Building
{
    public BuildingType BuildingType { get; protected set; }
    public string Address { get; set; }
    public decimal Price { get; set; }

    public Building()
    {
        BuildingType = BuildingType.General;
        Address = "Unknown";
    }

    public Building(string address, decimal price)
        : this()
    {
        Address = address;
        Price = price;
    }
}

The class is using an enum

enum BuildingType { None, General, Office, Apartment }

Now I want to create a child class Office which also has an overloaded constructor. This child class adds another property (Company). In this class, the BuildingType property should off course be set to Office. This is the code.

class Office : Building
{
    public string Company { get; set; }

    public Office()
    {
        BuildingType = BuildingType.Office;
    }

    public Office(string address, decimal price, string company)
        : base(address, price)
    {
        Company = company;
        // BuildingType = BuildingType.Office; // Don't wanna repeat statement
    }
}

What I want and why

I want the second constructor for the Office class to execute both the base(address, price) constructor as well as the default constructor of the Office class. I want to call the base(address, price) constructor so I don't have to repeat assigning all the properties of the base class. I want to call the default constructor of the Office class because it sets the BuildingType property to BuildingType.Office.

Now I know I can't using something like this.

public Office(string address, decimal price, string company)
    : base(address, price) this()

Am I doing something wrong?

I'm wondering if there's something wrong with my design that makes me want to call both base(address, price) and this(). Maybe I shouldn't be setting the BuildingType in the constructor but somewhere else? I've tried to introduce a field for this.

    public BuildingType BuildingType = BuildingType.General;

But then I can't do the same in the child class. I'd be hiding the BuildingType field in the base class so I'd have to use the new operator in the child class. I've tried making the BuildingType in the base class virtual, but a field can't be made virtual.

Creating something in the base constructor

In this simple example the default constructors only assign default values to some properties. But the Building constructor could als be creating a Foundation for the building, while the Office default constructor might create a... (can't think of something, but you get the idea). So then you'd still want to execute both default constructors.

Am I thinking in the wrong direction here?


Update

Based on Jon Skeet's answer and comments, here's my new code. I've changed constructor chaining from least specific to most specific. I've also added the BuildingType to the constructor of the Building class, made that constructor protected, and made the property setter private.

enum BuildingType { None, General, Office, Apartment }

class Building
{
    private const string DefaultAddress = "Unknown";

    public BuildingType BuildingType { get; private set; }
    public string Address { get; set; }
    public decimal Price { get; set; }

    #region Optional public constructors
    // Only needed if code other than subclass must 
    // be able to create a Building instance.
    // But in that case, the class itself can be abstract
    public Building() : this(DefaultAddress, 0m)
    {}

    public Building(string address, decimal price)
        : this(BuildingType.General, address, price) 
    {}
    #endregion

    protected Building(BuildingType buildingType)
        : this(buildingType, DefaultAddress, 0m) 
    {}

    protected Building(BuildingType buildingType,
        string address, decimal price)
    {
        BuildingType = buildingType;
        Address = address;
        Price = price;
    }
}

class Office : Building
{
    public string Company { get; set; }

    public Office() : this("Unknown Office", 0m, null) 
    {}

    public Office(string address, decimal price, string company)
        : base(BuildingType.Office, address, price)
    {
        Company = company;
    }
}

Can you (Jon Skeet or someone else) please comment on this revised version of the code?

One (minor) problem that isn't solved by this is that the default constructor for the Office class still needs to provide a default address ("Unknown Office" in the above code). I would still prefer to let the constructor of the base class decide on the address if one isn't specified. So this code still doesn't do exactly what I want.

I could probably solve that by not using constructor chaining in the derived class, but in stead have each of it's constructors directly call the base constructor. That would mean I'd change the default constructor of the Office class to

public Office() : base(BuildingType.Office)

That would work for this simple example, but if there's some method I'd like to execute on every instantiation of an Office, I'd have to call in in all constructors. That's why constructor chaining sounds like a better idea to me.

1

2 Answers 2

64

Your approach isn't the conventional one, which would solve the problem. Instead of making the more specific constructor (the one with lots of parameters) call the parameterless one, do things the other way round - make the parameterless one call the other, providing defaults. This typically leads to all the constructors bar one in each class calling one "primary" one (possibly indirectly, via others) and that "primary" constructor calls make the base constructor call.

class Office : Building
{
    public string Company { get; set; }

    public Office() : this(null, 0m, null)
    {
    }

    public Office(string address, decimal price, string company)
        : base(address, price)
    {
        Company = company;
        BuildingType = BuildingType.Office; // Don't wanna repeat statement
    }
}

... and the same in the base class:

class Building
{
    public BuildingType BuildingType { get; protected set; }
    public string Address { get; set; }
    public decimal Price { get; set; }

    public Building() : this("Unknown", 0m)
    {
    }

    public Building(string address, decimal price)
    {
        BuildingType = BuildingType.General;
        Address = address;
        Price = price;
    }
}

(I would seriously consider making the Building constructor include a BuildingType parameter, too.)

9
  • If I include a BuildingType parameter in the constructor, wouldn't that make it possible to create an instance of a Building that "thinks" it's an Office? Also, I suppose you mean this(null, 0, null)?
    – comecme
    Dec 10, 2010 at 20:20
  • class Office doesn't a constructor that takes one argument. You probably meant this(null, 0m, null)
    – Vadim
    Dec 10, 2010 at 20:20
  • @comecme: Not if you make that constructor protected so only inheritors can use it. Dec 10, 2010 at 20:22
  • 1
    @comecme: You can do that already, with a subclass. By all means make such a constructor protected, so that only a subclass gets to specify the building type. But then you can make it a private setter, and ensure you only set it once. Currently buildings can change type at will, which I suspect is not a good idea.
    – Jon Skeet
    Dec 10, 2010 at 20:23
  • 1
    @comecme: Yes, that looks better. I'd consider making the Building class abstract to start with, mind you.
    – Jon Skeet
    Dec 22, 2010 at 10:06
1

Create a private method that assigns the default values to all your properties and call them from each constructor seperately.

2
  • 2
    Off course that would help. However, to me this feels more like a workaround than an actual solution.
    – comecme
    Dec 10, 2010 at 20:26
  • 5
    Calling a method to assign values instead of chaining constructors will lead to problems if you ever decide to make one or more of those properties/member variables readonly. They can only be assigned in the constructor or during declaration. Not an issue in this case, but it does force you to change your public interface to the class if you decide to make this type of change in the future.
    – Jason Down
    Dec 11, 2010 at 3:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.