57

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.

comecme
  • 6,086
  • 10
  • 39
  • 67
  • possible duplicate of [Calling Overriden Constructor and Base Constructor in C#](http://stackoverflow.com/questions/335286/calling-overriden-constructor-and-base-constructor-in-c-sharp) – nawfal Feb 08 '14 at 11:49

2 Answers2

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

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 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 '10 at 20:20
  • class Office doesn't a constructor that takes one argument. You probably meant this(null, 0m, null) – Vadim Dec 10 '10 at 20:20
  • @comecme: Not if you make that constructor protected so only inheritors can use it. – Josh Sterling Dec 10 '10 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 '10 at 20:23
  • @Jon Skeet: A disadvantage of your answer might be that both the base class and the child class have to supply default values for address and price. Also, if Building would have even more overloads for the constructor, they all would have to call the same constructor with the same `"Unknown"`. Off course I can use a constant for that, but still... – comecme Dec 10 '10 at 21:03
  • @comecme: Personally I haven't found this to be a problem. Then again, I don't use inheritance that much, and don't tend to have lots of overloaded constructors... – Jon Skeet Dec 10 '10 at 21:16
  • Since this is my first question, I'm unsure what to do next. I can't post code in a comment. If I want to place modified code here, so you can see if that is better, what should I do? Post my own answer with the new code in it? Edit my question and add the new code? Something else? – comecme Dec 10 '10 at 21:29
  • I've updated the code in my question (I actually did that on 11 December). Would anybody like to comment on the `Update` part of my question? – comecme Dec 22 '10 at 10:02
  • 1
    @comecme: Yes, that looks better. I'd consider making the Building class abstract to start with, mind you. – Jon Skeet Dec 22 '10 at 10:06
1

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

XtremeBytes
  • 1,469
  • 8
  • 12
  • 2
    Off course that would help. However, to me this feels more like a workaround than an actual solution. – comecme Dec 10 '10 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 '10 at 03:24