Pros and cons of 'new' properties in C# / .Net?

Question

Considering the following sample code:

// delivery strategies
public abstract class DeliveryStrategy { ... }
public class ParcelDelivery : DeliveryStrategy { ... }
public class ShippingContainer : DeliveryStrategy { ... }

and the following sample Order class:

// order (base) class
public abstract class Order
{
    private DeliveryStrategy delivery;

    protected Order(DeliveryStrategy delivery)
    {
        this.delivery = delivery;
    }

    public DeliveryStrategy Delivery
    {
        get { return delivery; }
        protected set { delivery = value; }
    }
}

When i derive a new type of order class, it will inherit the Delivery property of type DeliveryStrategy.

Now, when it is given that CustomerOrders must be delivered using the ParcelDelivery strategy, we could consider 'new'ing the Delivery property in the CustomerOrder class:

public class CustomerOrder : Order
{
    public CustomerOrder()
        : base(new ParcelDelivery())
    { }

    // 'new' Delivery property
    public new ParcelDelivery Delivery
    {
        get { return base.Delivery as ParcelDelivery; }
        set { base.Delivery = value; }
    }
}

(The CustomerOrder obviously needs to ensure that is compatible (polymorph) with Order)

This allows direct usage of the ParcelDelivery strategy on CustomerOrder without the need for casting.

Would you consider using this pattern? why / why not?

Update: i came up with this pattern, instead of using generics, because i want to use this for multiple properties. I don't want to use generic type arguments for all these properties

Answer 1

You can use generics.

// order (base) class
public abstract class Order<TDeliveryStrategy> where TDeliveryStrategy : DeliveryStrategy
{
    private TDeliveryStrategy delivery;

    protected Order(TDeliveryStrategy delivery)
    {
        this.delivery = delivery;
    }

    public TDeliveryStrategy Delivery
    {
        get { return delivery; }
        protected set { delivery = value; }
    }
}

public class CustomerOrder : Order<ParcelDelivery>
{
    public CustomerOrder()
        : base(new ParcelDelivery())
    { }
}

Answer 2

I'd prefer to make the type generic:

public abstract class Order<TDelivery> where TDelivery : Delivery
{
    public TDelivery Delivery { ... }
    ...
}

public class CustomerOrder : Order<ParcelDelivery>
{
    ...
}

This ensures type safety at compile-time, rather than leaving it up to execution time. It also prevents the situation of:

CustomerOrder customerOrder = new CustomerOrder();
Order order = customerOrder;
order.Delivery = new NonParcelDelivery(); // Succeeds!

ParcelDelivery delivery = customerOrder.Delivery; // Returns null

Ouch.

I regard new as usually a last resort. It introduces added complexity both in terms of implementation and use.

If you don't want to go down the generic route, I'd introduce a genuinely new property (with a different name).

Answer 3

Is there any reason why you need to have the return type change? If there isn't then I would suggest just making the Delivery property virtual so it has to be defined by inherited classes instead:

public abstract class Order
{
    protected Order(DeliveryStrategy delivery)
    {
        Delivery = delivery;
    }

    public virtual DeliveryStrategy Delivery { get; protected set; }
}

public class CustomerOrder : Order
{
    public CustomerOrder()
        : base(new ParcelDelivery())
    { }

    public DeliveryStrategy Delivery { get; protected set; } 
}

If you do require the change in return type, then I would wonder why you would need that drastic of a behavior change on the return type. Regardless, if that is so, then this won't work for you.

So to directly answer your question, I would only use the pattern you've described if it is required that the return type be different from the base class, and very sparingly (I would analyze my object model to see if there's something else i could do first). If that is not the case, then I would use the pattern I've described above.

Answer 4

I think this is a good pattern. It makes it easier to explicitly use derived types by removing the need to cast the result, and it doesn't 'break' the base class behavior. Actually, a similar pattern is used in some classes in the BCL, for instance look at the DbConnection class hierarchy :

• DbConnection.CreateCommand() returns a DbCommand
• SqlConnection.CreateCommand() hides the base implementation using 'new' to return a SqlCommand.
• (other DbConnection implementations do the same)

So, if you manipulate the connection object through a DbConnection variable, CreateCommand will return a DbCommand ; if you manipulate it through a SqlConnection variable, CreateCommand will return a SqlCommand, avoiding the cast if you're assigning it to a SqlCommand variable.

Answer 5

Using the 'new' keyword to hide writable properties from the base class is a bad idea in my opinion. The new keyword allows you to hide a member of a base class in a derived class, rather than override it. This means that calls to such members using a base-class reference still access the base class code, not derived class code. C# has the 'virtual' keyword, which allows derived classes to actually override the implementation, rather than simply hiding it. There's a reasonably good article here that talks about the differences.

In your case, it looks like you are trying to use method hiding as a way of introducing property covariance to C#. However, there are problems with this approach.

Often, the value of having a base class is to allow users of your code to treat types polymorphically. What happens with your implementation if someone sets the Delivery property using a reference to the base class? Will the derived class break if the Delivery property is NOT an instance of ParcelDelivery? These are the kinds of questions you need to ask yourself about this choice of implementation.

Now, if the Delivery property in the base class did not supply a setter, then you have a slightly different situation. Users of the base class can only retrieve the property and not set it. Since you route your property access back to the base class, access through the base class continues to work. However, if your derived class is not sealed, classes that inherit from it could introduce the same types of problems by hiding the Delivery property with a version of their own.

As some of the other posts have already mentioned, you could use generics as a way to achieve different Delivery property types. Jon's example is pretty good at demonstrating this. There is one problem with the approach of generics, if you need to derive from CustomerOrder and change the Delivery property to a new type.

There is an alternative to generics. You need to consider whether you really want a settable property in your case. If you get rid of the setter on the Delivery property, the issues introduced by using the Order class directly go away. Since you set the delivery property using constructor parameters, you can guarantee that all orders have the right type of strategy.

Answer 6

Consider this approach:

public interface IOrder
{
    public DeliveryStrategy Delivery
    {
        get;
    }
}

// order (base) class
public abstract class Order : IOrder
{
    protected readonly DeliveryStrategy delivery;

    protected Order(DeliveryStrategy delivery)
    {
        this.delivery = delivery;
    }

    public DeliveryStrategy Delivery
    {
        get { return delivery; }
    }
}

then use

public class CustomerOrder : Order
{
    public CustomerOrder()
        : base(new ParcelDelivery())
    { }

    public ParcelDelivery Delivery
    {
        get { return (ParcelDelivery)base.Delivery; }
    }


    DeliveryStrategy IOrder.Delivery
    {
        get { return base.Delivery}
    }
}

This is still far from perfect (your example doesn't show why the base class needs to know about the delivery strategy at all and it would make much more sense to be generic with a constraint but this at least allows you to use the same name for the property and get type safety.

The as in your example was pointless, if something is ever not the right type you should not mask it with null you should throw as your invariant has been violated.

readonly fields are always preferable where possible. They make the immutability clear.

Answer 7

Your solution does not do what you think it does. It appears to work but it is not calling your "new" method. Consider the following changes to your code to add some output to see which method is called:

// order (base) class
public abstract class Order
{
    private DeliveryStrategy delivery;

    protected Order(DeliveryStrategy delivery)
    {
        this.delivery = delivery;
    }

    public DeliveryStrategy Delivery
    {
        get 
        {
            Console.WriteLine("Order");
            return delivery; 
        }
        protected set { delivery = value; }
    }
}

public class CustomerOrder : Order
{
    public CustomerOrder()
        : base(new ParcelDelivery())
    { }

    // 'new' Delivery property
    public new ParcelDelivery Delivery
    {
        get 
        {
            Console.WriteLine("CustomOrder");
            return base.Delivery as ParcelDelivery; 
        }
        set { base.Delivery = value; }
    }
}

Then the following snippet of code to actually use your CustomOrder class:

Order o = new CustomerOrder();
var d = o.Delivery;

Would output the "Order". The new method specifically breaks polymorphism. It creates a new Delivery property on CustomOrder that is not part of the Order base class. So when you use your CustomOrder as if it were an Order, you do not call the new Delivery property because that only exists on CustomOrder and is not part of the Order class.

What you're trying to do is override a method that isn't overridable. If you mean the property to be overridable, make it abstract:

// order (base) class
public abstract class Order
{
    private DeliveryStrategy delivery;

    protected Order(DeliveryStrategy delivery)
    {
        this.delivery = delivery;
    }

    public abstract DeliveryStrategy Delivery
    {
        get { return delivery; }
        protected set { delivery = value; }
    }
}

public class CustomerOrder : Order
{
    public CustomerOrder()
        : base(new ParcelDelivery())
    { }

    public override ParcelDelivery Delivery
    {
        get { return base.Delivery as ParcelDelivery; }
        set { base.Delivery = value; }
    }
}