Immutable pure data classes with public setters on properties

Question

I'm putting together some classes as a model for some information (that I'm currently pulling from a website).

The classes are implemented in C# - because in the current version of F# there are no autoimplemented properties. The logic to fill these classes from the website is going to be writen in F#. (Because I like F#, and it works nice for webscraping.)

So since I'm working functionally (I seems to always work functionally there days even in C#). I don't want these objects to be mutable. Setting mutable fields is ugly in F# (and for good reason, mutable objects are evil).

So all there data fields have only private setters but this makes my constructors long. I planned to be using named arguments in them anyway, but still 5 arguments is a lot. And I don't think F# supports object property initialisers anyway.

public class PopularitySplitClassOptionSet : IClassOptionsSet
{
    public PopularitySplitClassOptionSet (string description, IEnumerable<ClassOption> popularClasses, IEnumerable<ClassOption> unpopularClasses, int reqPreferences, int minUnpopularPreferences)
    {
        PopularClasses  = popularClasses;
        UnpopularClasses = unpopularClasses;
        RequiredPrefereces = reqPreferences;
        MinUnpopularPrefereces = minUnpopularPreferences;
    }


    public IEnumerable<ClassOption> Classes
    {
        get 
        {
            return PopularClasses.Concat(UnpopularClasses);
        }
    }


    public int RequiredPrefereces { get; private set; }
    public int MinUnpopularPrefereces { get; private set; }

    public int MaxPopularPrefereces { 
        get 
        {
            return RequiredPrefereces - MinUnpopularPrefereces;
        }
    }

    public  IEnumerable<ClassOption> PopularClasses { get; private set; }
    public  IEnumerable<ClassOption> UnpopularClasses { get; private set; }

}

Edit: Is this good practice? Am I thinking this right?

Also: I wonder if:

public readonly int RequiredPrefereces { get; private set; }

would be better? Or is that not actually a thing you can do?

Related questions:

• C#, immutability and public readonly fields
• How to avoid “too many parameters” problem in API design?

Answer 1

This is what I program when I want "immutable" classes in C#. You do have an access leak right now though, in that PopularClasses and UnpopularClasses both set and get the underlying object, allowing modification of that object, thus making the class immutable. (This is a bigger problem for set than get, as get would require figuring out which collection type is actually being held. Set you already know because you set it.)

List<string> popularClasses = new List<string>;
popularClasses.add("hello");

pscos = new PopularitySplitClassOptionSet(..., popularClasses, ...);
popularClasses.add("sup"); // This is now added to pscos.

List<string> getAlsoLeaks = (List<string>)(pscos.PopularClasses);
getAlsoLeaks.add("yo"); // This is also added.

You will need to copy on get and set to avoid this. Usually I use LinkedList<T> for this purpose, unless I will need random access, in which case I use List<T>. Since you're only currently using IEnumerable<T>, I'm assuming that random access isn't an issue and using LinkedList<T>.

private LinkedList<ClassOption> popularClasses;
public IEnumerable<ClassOption> PopularClasses
{
    get
    {
        foreach (ClassOption classOption in list.popularClasses)
            yield return classOption;
        // OR:
        // return new LinkedList<ClassOption>(this.popularClasses);
    }
    set
    {
        this.popularClasses = new LinkedList<ClassOption>(value);
    }
}

Answer 2

Here's a take where the collection objects are passed back as read-only to the caller and the member variables are completely immutable:

public sealed class PopularitySplitClassOptionSet : IClassOptionsSet
{
    private readonly string description;

    private readonly IEnumerable<ClassOption> popularClasses;

    private readonly IEnumerable<ClassOption> unpopularClasses;

    private readonly int requiredPrefereces;

    private readonly int minUnpopularPrefereces;

    private readonly IEnumerable<ClassOption> classes;

    private readonly int maxPopularPrefereces;

    public PopularitySplitClassOptionSet(
        string description,
        IEnumerable<ClassOption> popularClasses,
        IEnumerable<ClassOption> unpopularClasses,
        int reqPreferences,
        int minUnpopularPreferences)
    {
        if (description == null)
        {
            throw new ArgumentNullException("description");
        }

        if (popularClasses == null)
        {
            throw new ArgumentNullException("popularClasses");
        }

        if (unpopularClasses == null)
        {
            throw new ArgumentNullException("unpopularClasses");
        }

        if (minUnpopularPreferences > reqPreferences)
        {
            throw new ArgumentOutOfRangeException(
                "minUnpopularPreferences",
                minUnpopularPreferences,
                "The minimum number of unpopular preferences may not exceed the number of required preferences.");
        }

        this.description = description;
        this.popularClasses = popularClasses.ToList().AsReadOnly();
        this.unpopularClasses = unpopularClasses.ToList().AsReadOnly();
        this.requiredPrefereces = reqPreferences;
        this.minUnpopularPrefereces = minUnpopularPreferences;

        this.classes = this.popularClasses.Concat(this.unpopularClasses).ToList().AsReadOnly();
        this.maxPopularPrefereces = this.requiredPrefereces - this.minUnpopularPrefereces;
    }

    public string Description
    {
        get
        {
            return this.description;
        }
    }

    public IEnumerable<ClassOption> Classes
    {
        get
        {
            return this.classes;
        }
    }

    public int RequiredPrefereces
    {
        get
        {
            return this.requiredPrefereces;
        }
    }

    public int MinUnpopularPrefereces
    {
        get
        {
            return this.minUnpopularPrefereces;
        }
    }

    public int MaxPopularPrefereces
    {
        get
        {
            return this.maxPopularPrefereces;
        }
    }

    public IEnumerable<ClassOption> PopularClasses
    {
        get
        {
            return this.popularClasses;
        }
    }

    public IEnumerable<ClassOption> UnpopularClasses
    {
        get
        {
            return this.unpopularClasses;
        }
    }
}

I think a five or six line constructor is completely acceptable. In fact, my revision includes some sanity checks on the parameters. The second revision does the calculations in the constructor since the values are immutable (no need to do those concats and subtraction in the properties).