1
\$\begingroup\$

I have a Job class that represents jobs in a manufacturing system. The idea is that a job requires some quantity of certain materials, and it outputs some quantity of materials as well. It also has an amount of time it takes in minutes.

I'm representing the material requirements/outputs using Dictionary objects which map a MaterialType enum to the quantity required for that material. In other classes, these Jobs will be stored using a HashSet, so I overrode the Equals() and GetHashCode() methods so they determine equality of two Jobs based on the contents of both dictionaries.

I'm wondering whether my implementation is a good way to go about this. There's some debate on this StackOverflow post about best practices for hash codes when objects contain collections. I'm under the impression that since my object is immutable, basing the hash code on the contents of the dictionaries should be fine. But in order to both enforce immutability and keep the init accessors, I had to jump through a few hoops with backing fields and whatnot. So I'm also wondering if this whole implementation is overengineered. Are there ways it can be simplified?

Here's my Job class:

public class Job
{
    //backing fields default to new() or else i have to make them nullable.
    //i think this is because we have no constructor, only init accessors.
    //ultimately, i believe it doesn't matter since the properties are required to be initialized
    private readonly Dictionary<MaterialType, int> _materialIn = new();
    private readonly Dictionary<MaterialType, int> _materialOut = new();

    //public properties return a copy of the dictionary, keeping a Job immutable
    public required Dictionary<MaterialType, int> MaterialIn
    { 
        get => new(_materialIn); 
        init => _materialIn = value;
    }
    public required Dictionary<MaterialType, int> MaterialOut 
    { 
        get => new(_materialOut); 
        init => _materialOut = value; 
    }

    public required int TimeMinutes { get; init; }

    public override bool Equals(object? obj)
    {
        if (obj is not Job other)
        {
            return false;
        }

        //check if the dictionary references are the same. if that's false, then check their
        //counts and then their values
        return (_materialIn == other.MaterialIn && _materialOut == other.MaterialOut) ||
            (_materialIn.Count == other.MaterialIn.Count &&
            _materialOut.Count == other.MaterialOut.Count &&
            !_materialIn.Except(other.MaterialIn).Any() &&
            !_materialOut.Except(other.MaterialOut).Any());
    }

    public static bool operator ==(Job? o1, Job? o2)
    {
        if (o1 is null && o2 is null)
        {
            return true;
        }
        else if (o1 is null || o2 is null)
        {
            return false;
        }

        return o1.Equals(o2);
    }

    public static bool operator !=(Job? o1, Job? o2)
    {
        return !(o1 == o2);
    }

    public override int GetHashCode()
    {
        int hashCode = 0;

        //iterate through the keys in order, using both the key and value to build the hash code
        foreach(MaterialType type in _materialIn.Keys.Order())
        {
            hashCode ^= type.GetHashCode();
            hashCode = (hashCode << 7) | (hashCode >> (32 - 7));

            hashCode ^= _materialIn[type];
            hashCode = (hashCode << 7) | (hashCode >> (32 - 7));
        }

        foreach(MaterialType type in _materialOut.Keys.Order())
        {
            hashCode ^= type.GetHashCode();
            hashCode = (hashCode << 7) | (hashCode >> (32 - 7));

            hashCode ^= _materialOut[type];
            hashCode = (hashCode << 7) | (hashCode >> (32 - 7));
        }

        return hashCode;
    }
}

//including enum here with a couple test values for example purposes
public enum MaterialType
{
    ROLL_212,
    ROLL_210,
    SIDE_PLATE_QC,
    TORSO_PLATE_QC,
    SHEETED_210,
    SHEETED_212,
    BOOK_SIDE,
    BOOK_TORSO
}

And for testing purposes, I have this top-level code. It produces the results I expect, with Jobs that have equivalent requirements/outputs producing the same hash code, even if they're not using the exact same reference. The immutability and equality checks seem to work as well.

using MaterialDict = System.Collections.Generic.Dictionary<Manufacturing_Simulation.NonThreaded.MaterialType, int>;

MaterialDict test1 = new()
{
    { MaterialType.ROLL_212, 1 },
    { MaterialType.ROLL_210, 2 }
};

MaterialDict test2 = new()
{
    { MaterialType.ROLL_212, 1 },
    { MaterialType.ROLL_210, 2 }
};

//test1 and test2 should produce the same hash code when added to a job, even though they're
//separate objects
Job job1 = new() { MaterialIn = test1, MaterialOut = test1, TimeMinutes = 1 };
Job job2 = new() { MaterialIn = test2, MaterialOut = test2, TimeMinutes = 1 };

Console.WriteLine("Job 1: " + job1.GetHashCode());
Console.WriteLine("Job 2: " + job2.GetHashCode());

//job3 should also produce the same hash code, using a different combination of the same objects
Job job3 = new() { MaterialIn = test1, MaterialOut = test2, TimeMinutes = 1 };

Console.WriteLine("Job 3: " + job3.GetHashCode());

MaterialDict test3 = new()
{
    { MaterialType.SIDE_PLATE_QC, 20 },
    { MaterialType.TORSO_PLATE_QC, 20 }
};

Job job4 = new() { MaterialIn = test3, MaterialOut = test3, TimeMinutes = 1 };

//but job4 should have a totally different hash code
Console.WriteLine("Job 4: " + job4.GetHashCode());

//what if the dictionaries are in different orders?
MaterialDict test4 = new()
{
    { MaterialType.SHEETED_210, 1 },
    { MaterialType.SHEETED_212, 1 }
};

MaterialDict test5 = new()
{
    { MaterialType.BOOK_SIDE, 10 },
    { MaterialType.BOOK_TORSO, 30 }
};

//these two objects should NOT have the same hash code
Job job5 = new() { MaterialIn = test4, MaterialOut = test5, TimeMinutes = 1 };
Job job6 = new() { MaterialIn = test5, MaterialOut = test4, TimeMinutes = 1 };

Console.WriteLine("Job 5: " + job5.GetHashCode());
Console.WriteLine("Job 6: " + job6.GetHashCode());

//and just to check...
if(job5 == job6)
{
    Console.WriteLine("Job 5 and 6 are the same (this is incorrect)");
}
else
{
    Console.WriteLine("Job 5 and 6 are not the same (this is correct)");
}

//and one last check, what if dictionaries have the same keys and values, but they were inserted in
//a different order? this shouldn't matter
MaterialDict test6 = new()
{
    { MaterialType.SHEETED_210, 1 },
    { MaterialType.SHEETED_212, 1 }
};

MaterialDict test7 = new()
{
    { MaterialType.BOOK_TORSO, 30 },
    { MaterialType.BOOK_SIDE, 10 }
};

Job job7 = new() { MaterialIn = test6, MaterialOut = test7, TimeMinutes = 1 };

Console.WriteLine("Job 7: " + job7.GetHashCode());

if(job5 == job7)
{
    Console.WriteLine("Job 5 and 7 are the same (this is correct)");
}
else
{
    Console.WriteLine("Job 5 and 7 are not the same (this is incorrect)");
}

//we should not be allowed to do this and have it affect the job itself
job7.MaterialIn.Add(MaterialType.SIDE_PLATE_QC, 4);

Console.WriteLine("Job 7 (after modify attempt): " + job7.GetHashCode());

if (job5 == job7)
{
    Console.WriteLine("Job 5 and 7 are still the same (this is correct)");
}
else
{
    Console.WriteLine("Job 5 and 7 are now not the same (this is incorrect)");
}
\$\endgroup\$
2
  • \$\begingroup\$ I imagine this code is nice enough. Here's a remark I'll just throw out there. Suppose you have an immutable type for which you know how to (1) sort the keys, and (2) serialize it to string, bytes, something like that. Double your RAM usage by hanging on to the serialized form, and then your problem is solved, right? \$\endgroup\$
    – J_H
    Jul 28, 2023 at 22:55
  • 1
    \$\begingroup\$ Which .NET version are you using? Can't you use the HashCode struct? \$\endgroup\$ Jul 31, 2023 at 13:57

1 Answer 1

1
\$\begingroup\$

Use SortedDictionary to replace .Order()

Since you sort the keys of both dictionaries to calculate the hashcode of a Job you can save some time doing this by using the SortedDictionary and getting rid of the Order() calls or just use a filed to store the hashcode. It doesn't change anyway so why recalculate it every time?

Use ImmutableSortedDictionary

You want to have immutable Jobs so instead of copying the internal dictionaries on get, use the ImmutableSortedDictionary and they will turn both properties into oneliners and make both their backing fields unnecessary:

public required ImmutableSortedDictionary<MaterialType, int> MaterialIn { get; init; }
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.