C# Design, many properties, complex constructor and equality

Question

I'm still a relative novice with OOP, so I am still trying to understand some of the best practices for how to design "good" objects. I know this question is probably a rather subjective, which is why I am here rather than on stackoverflow.

I am designing an object that will have 20 separate string properties, none of which are required. These constraints are fixed and cannot be changed as it is an implementation of an outside data source that I have no control over.

The design problem I am running into is how to effectively construct an instance of the object and be able to compare equality by comparing each field without violating normal best practices.

• A 20 argument constructor seems very wrong, especially since every field is a string, and there are cases where many of them will be null. I could use the optional arguments in C#4 to reduce the number of arguments that are actually called, but since everything is a string, I would have to use named parameters when calling the constructor, and that seems that it would be even uglier.

• I have thought about trying to group the properties into smaller classes, but the most logical grouping still has an object with 10 properties, and at least half of the remaining properties have no logical grouping.

• I could give each property a public setter, but that would make the object mutable, and thus complicate matters for overriding GetHashCode(). And I really don't need the object to be mutable. Once it is built, there should never be a reason to change it, but I don't want to rely on that because who knows what someone will try to do with it.

• Implementing IEquatable<T> is a possibility for the equality test, but my understanding is it is recommended to also override Equals(object obj) and GetHashCode() when implementing IEquatable so all Equals methods have the same behavior.

Is this an edge case where overriding Equals without overriding GetHashCode should be done? I don't intend on using this object as a key in a dictionary or as a member of a HashSet, but I can't predict what someone else would try to do in the future.

Or should I just abandon the idea of doing an equality comparison on all of the properties with Equals and implement my own Compare() method?

Or is there a better method for building the object that would allow it to still be immutable without having to pass all of the arguments through a constructor or a method?

EDIT: In response to Sign's comment, the purpose of the object is ultimately to be able to compare the data from the outside data source to our internal data to ensure it is accurate. Both data sets contain address information, but they are broken up differently.

The internal data, is generally stored as a typical address (Street, City, State, Postal Code, County, Country), but there are variations when you have a suite/apartment # or a floor in a building.

The external data is broken down even further into smaller pieces.

So this object is supposed to be a common link between the 2 data sources and allow the user to compare like fields for equality.

EDIT #2: Ultimately, I chose Doc Brown's answer because a modified version of his dictionary idea (with Enums as Keys instead of strings) was the simplest way to go without having a messy constructor and still provide the immutability I was hoping to have. In the long term, I am going to experiment with some of Oliver's suggestions as some of his edits did have a lot to offer.

Answer 1

If you require your object to be immutable, there is obviously no other way than providing the 20 strings through the constructor. And if you want to be able to leave some of the strings out, you must say which arguments you are providing and which are not, which leads you to some form of named parameters. Of course, besides the possibilities you mentioned by yourself, you can also

• provide the 20 arguments by a list of strings
• provide them by a dictionary (key=>value, where "key" is the attribute name)
• provide them by an object of a helper class, which has the same 20 properties with getters and setters, so this one won't be immutable

It may also be a good idea to implement Equals and GetHashCodeby utilizing reflection, looping over all public string properties of your class.

Answer 2

I think a builder could be of use here. It has the benefit of keeping your value class immutable, but creation is no longer such a huge mess since you only need to specify what you want and are free to specify defaults for the others.

Of course this can be extended to include guards and other stuff, but this should give a general idea:

public class Big
{
    public readonly string A;
    public readonly string B;
    public readonly string C;
    public readonly string D;
    public readonly string E;

    public Big(string a, string b, string c, string d, string e)
    {
        A = a;
        B = b;
        C = c;
        D = d;
        E = e;
    }
}

public class BigBuilder
{
    private string _a = string.Empty;
    private string _b = string.Empty;
    private string _c = string.Empty;
    private string _d = string.Empty;
    private string _e = string.Empty;

    public BigBuilder WithA(string a)
    {
        _a = a;
        return this;
    }

    public BigBuilder WithB(string b)
    {
        _b = b;
        return this;
    }

    public BigBuilder WithC(string c)
    {
        _c = c;
        return this;
    }

    public BigBuilder WithD(string d)
    {
        _d = d;
        return this;
    }

    public BigBuilder WithE(string e)
    {
        _e = e;
        return this;
    }

    public Big Build()
    {
        return new Big(_a, _b, _c, _d, _e);
    }
}

public class Test
{
    public static void Run()
    {
        new BigBuilder()
            .WithC("6")
            .WithD("hello")
            .Build();
    }
}

Answer 3

There is absolutely no problem with implementing GetHashCode() whether your object is immutable or not. If it is immutable, the hashcode can be precomputed; if it is mutable, the hashcode needs to be recalculated each time the GetHashCode() method is invoked. And since your object consists of nothing but 20 strings, which are immutable, and therefore their hashcodes are already computed, computing your hashcode is close to trivial. Just multiply the hashcodes of your non-null string members together, or search stackoverflow for some slightly better hashcode algorithm.

Or is there a better method for building the object that would allow it to still be immutable without having to pass all of the arguments through a constructor or a method?

I am a fanatic proponent of immutable objects, so I would advice you to make yours immutable. And I would not pass a dictionary or a list to the constructor; I would pass 20 separate arguments, there is nothing wrong with that, and there is nothing gained doing it any other way. Any other way is bound to move some of the checking that the compiler will do for you at compile time, to checking that you will have to do yourself at runtime: always a bad idea.

Is this an edge case where overriding Equals without overriding GetHashCode should be done? I don't intend on using this object as a key in a dictionary or as a member of a HashSet, but I can't predict what someone else would try to do in the future.

For IEquatable<T> it is true that you do not, strictly speaking, need to implement GetHashCode(), except to avoid the warning that you will receive if you don't, but good practice is to always implement GetHashCode(), precisely because someone might try to use it in the future.

Note: Since you will be making your object immutable, (won't you?) you can precompute your hashcode, and store it with the object, so that the Equals() method can first compare hashcodes and return immediately if they do not match, rather than always doing a full comparison. In practice you do not need to do tricks like that unless you have already established that you have a performance issue, but it is a trick worth keeping in mind.

Or should I just abandon the idea of doing an equality comparison on all of the properties with Equals and implement my own Compare() method?

I do not know what you mean by this question. You will be implementing your own Equals() method, and it will be a series of if( !Object.Equals( MyField1, other.MyField1 ) ) return false;

Answer 4

Apparently, you are not interested in individual properties; you just want to compare a bunch of fields. Therefore, a dictionary or a list of fieldname/value pairs would seem very appropriate to me. It allows you to do the comparisons in a loop instead of having to treat each field individually. It is also a universal approach as you can apply it to different sets of properties. In addition, it is more flexible as you will not have to change the algorithms if you add new fields.

I would not stick to the immutable approach. Instead, give your class an Add-method and forget the constructor with the endless parameter list.

public class FieldComparer : IEquatable<FieldComparer>
{
    private Dictionary<string,string> _fieldDict = new Dictionary<string,string>();

    public void Add(string fieldName, string value) {
        fieldDict.Add(fieldName, value);
    }

    // The magic comes here
}

---

EDIT: As an answer to your concern, I have two suggestions for different approaches:

---

1

In the class described above, supplement individual properties for fields that you want to handle individually:

public string LastName {
    get {
        string last_Name;
        _fieldDict.TryGet("LastName", out lastName);
        return lastName;
    }
    set {
        _fieldDict["LastName"] = value;
    }
}

---

2

Use properties in the first place and make them available through an indexer in order to be able to loop through them:

public string FistName { get; set; }
public string LastName { get; set; }
public string City { get; set; }

public const int PropertyCount = 3;

public string this[int i]
{
    get
    {
        switch (i) {
            case 0: 
                return FirstName;
            case 1: 
                return LastName;
            case 2: 
                return City;
            default:
                throw new ArgumentNullException("Index out of bounds.");
        }
    }
    set
    {
        switch (i) {
            case 0: 
                FirstName = value;
                break;
            case 1: 
                LastName = value;
                break;
            case 2: 
                City = value;
                break;
            default:
                throw new ArgumentNullException("Index out of bounds.");
        }
    }
}

(The setter of the indexer might not be necessary for your purpose.) You can then access the properties like this:

var comp = new FieldComparer();
comp.FistName = "John";
Console.WriteLine(comp[0]); // -> John

---

EDIT: The solution #2 has the disadvantage, that we lose the universality of the approach. We could restore it, if we implemented the indexer and PropertyCount from an interface. (We also would have to convert PropertyCount to a read-only property). The Comparison would then happen in a separate class.

public interface IPropertyIndexer
{
    string this[int i] { get; }
    int PropertyCount { get; }
}

Instead, I will show a complete solution using an enumerator instead of an indexer. Both approaches are comparable. The enumerator version using yield return is a bit shorter than the indexer version using a switch statement.

---

3

public interface IPropertyEnumerator
{
    IEnumerator<string> AllValues { get; }
}

public class Address : IPropertyEnumerator
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string City { get; set; }

    public IEnumerator<string> AllValues
    {
        get
        {
            yield return FirstName;
            yield return LastName;
            yield return City;
        }
    }
}

public class Customer : IPropertyEnumerator
{
    public string Name { get; set; }
    public string CustNo { get; set; }

    public IEnumerator<string> AllValues
    {
        get
        {
            yield return Name;
            yield return CustNo;
        }
    }
}

public static class PropertyComparer
{
    public static bool Equals<T>(T x, T y)
        where T : IPropertyEnumerator
    {
        var enum1 = x.AllValues;
        var enum2 = y.AllValues;
        while (enum1.MoveNext()) {
            if (!enum2.MoveNext() || enum1.Current != enum2.Current) {
                return false;
            }
        }
        return !enum2.MoveNext(); // true if number of properties equal
    }
}

public class Example
{
    public void Compare()
    {
        var a1 = new Address { FirstName = "John", LastName = "Doe", City = "New York" };
        var a2 = new Address { FirstName = "John", LastName = "Doe", City = "New York" };
        Console.WriteLine(PropertyComparer.Equals(a1, a2));

        var c1 = new Customer { CustNo = "A123", Name = "Miller" };
        var c2 = new Customer { CustNo = "B777", Name = "Stanley" };
        Console.WriteLine(PropertyComparer.Equals(c1, c2));
    }
}

Answer 5

It seems like you want two constructors one for each type of input data. If that data is already contained in an object you could take that in. The Dictionary constructor doesn't seem like a good idea since an arbitrary set of data is not going to be sufficient to do anything with(an object with just a country and a street name would do no good).

Answer 6

Depends on requirements of course... By efficient you mean execution speed, or dev speed / clarity?

Here is another possibility:

Have your constructor take a class object as it's only parameter. Certainly this "can" be easier to maintain and all the messy work to make it can be done elsewhere. This is definitely advisable where clarity is more important so that every field has a logical name and it's not meshed into a dictionary that may not get built properly. Who knows who will be working on your code next ;) Or maybe it will be you 2 years from now and you can save time and mistakes? But is that more important than execution speed? For me clarity trumps insignificant speed differences, up to a point where the accumulation is not insignificant of course ;)