C# Auto Property - Is this 'pattern' best practice?

Question

I seem to be using this sort of pattern in my code a lot , I know that it is not a simple Autoproperty any more as that would be:

  public IList<BCSFilter> BCSFilters { get; set; }

The code I have been using is this:

    private IList<BCSFilter> _BCSFilters;

    /// <summary>
    /// Gets or sets the BCS filters.
    /// </summary>
    /// <value>The BCS filters.</value>
    public IList<BCSFilter> BCSFilters
    {
        get
        {
            if (_BCSFilters == null)
            {
                _BCSFilters = new List<BCSFilter>();
            }

            return _BCSFilters;
        }
        set
        {
            _BCSFilters = value;
        }
    }

This is so I can just do MainClass.BCSFilters and not worry about needing to instantiate the List in the consuming code. Is this a 'normal' pattern \ the correct way to do this?

I couldn't find a duplicate question

Answer 1

This is a technique that I use a lot myself. This can also help save memory resources since it doesn't instantiate the List<> object unless the objects property is actually being used within the consuming code. This uses a "Lazy Loading" technique.

Also, the "Lazy Loading" technique that you listed isn't Thread Safe. If there happens to be multiple calls simultaneously to the property you could end up having multiple calls setting the property to a new List<> object, consequentially overwriting any existing List values with a new, empty List<> object. To make the Get accessor Thread Safe you need to use the Lock statement, like so:

private IList<BCSFilter> _BCSFilters;

// Create out "key" to use for locking
private object _BCSFiltersLOCK = new Object();

/// <summary>
/// Gets or sets the BCS filters.
/// </summary>
/// <value>The BCS filters.</value>
public IList<BCSFilter> BCSFilters
{
    get
    {
        if (_BCSFilters == null)
        {
            // Lock the object before modifying it, so other
            // simultaneous calls don't step on each other
            lock(_BCSFiltersLOCK)
            {
                if (_BCSFilters == null)
                }
                    _BCSFilters = new List<BCSFilter>();
                }
            }
        }

        return _BCSFilters;
    }
    set
    {
        _BCSFilters = value;
    }
}

However, if you'll always need the List<> object instantiated it's a little simpler to just create it within the object constructor and use the automatic property instead. Like the following:

public class MyObject
{
    public MyObject()
    {
        BCSFilters = new List<BCSFilter>();
    }

    public IList<BCSFilter> BCSFilters { get; set; }
}

Additionally, if you leave the "set" accessor public then the consuming code will be able to set the property to Null which can break other consuming code. So, a good technique to keep the consuming code from being able to set the property value to Null is to set the set accessor to be private. Like this:

public IList<BCSFilter> BCSFilters { get; private set; }

A related technique is to return an IEnumerable<> object from the property instead. This will allow you to replace the List<> type internally within the object at any time and the consuming code will not be affected. To return IEnumerable<> you can just return the plain List<> object directly since it implements the IEnumerable<> interface. Like the following:

public class MyObject
{
    public MyObject()
    {
        BCSFilters = new List<BCSFilter>();
    }

    public IEnumerable<BCSFilter> BCSFilters { get; set; }
}