Handling overwriting of unique Dictionary values

Question

I am working in C# to write a scheduling application for operations in a factory. I have a custom class Workday, with properties to represent a Workday for the operations in the shop. I also have a class Timeline that contains a dictionary of DateTimes as keys (using DateTime.Date to make sure we don't have two Workdays on the same date) and a Workday with the same Date as Key.Date as the value. When writing getters/setters for the Dictionary in the Timeline class, is it better to allow for overwriting a Workday, or is it preferred to clear the properties of the Workday and repopulate those values with new data?

public class Workday
{
    private Dictionary<ShopOperation, List<BOMProject>> OperationProjectPairs = new Dictionary<ShopOperation, List<BOMProject>>();
    public DateTime DayStart { get; set; }
    public DateTime DayEnd { get; set; }

    public void SetProjects(ShopOperation key, List<BOMProject> value)
    {
        if (OperationProjectPairs.ContainsKey(key))
        {
            OperationProjectPairs[key] = value;
        }
        else
        {
            OperationProjectPairs.Add(key, value);
        }
    }

    public List<BOMProject> GetProjects(ShopOperation key)
    {
        List<BOMProject> result = null;

        if (OperationProjectPairs.ContainsKey(key))
        {
            result = OperationProjectPairs[key];
        }

        return result;
    }
}

Timeline with getter that allows for full overwrite:

public class Timeline
{
    private Dictionary<DateTime, Workday> ScheduledDays = new Dictionary<DateTime, Workday>();

    public Workday GetWorkday( DateTime date )
    {
        Workday result = null;

        foreach( KeyValuePair<DateTime, Workday> pair in ScheduledDays)
        {
            if( pair.Key.Date == date.Date)
            {
                result = pair.Value;
            }
        }

        return result;
    }

    public void SetWorkday( DateTime key, Workday value)
    {
        bool scheduled = false;

        foreach( KeyValuePair<DateTime, Workday> pair in ScheduledDays)
        {
            if( pair.Key.Date == value.DayStart.Date)
            {
                ScheduledDays[key] = value;
                scheduled = true;
                break;
            }
        }

        if(!scheduled)
        {
            ScheduledDays.Add(key, value);
        }
    }
}

Timeline that will throw an exception if you try to overwrite a Workday:

public class Timeline
{
    private Dictionary<DateTime, Workday> ScheduledDays = new Dictionary<DateTime, Workday>();

    public Workday GetWorkday( DateTime date )
    {
        Workday result = null;

        foreach( KeyValuePair<DateTime, Workday> pair in ScheduledDays)
        {
            if( pair.Key.Date == date.Date)
            {
                result = pair.Value;
            }
        }

        return result;
    }

    public void SetWorkday( DateTime key, Workday value) Throws WorkdayOverwriteException
    {
        foreach( KeyValuePair<DateTime, Workday> pair in ScheduledDays)
        {
            if( pair.Key.Date == value.DayStart.Date)
            {
                throw new WorkdayOverwriteException();
            }
        }

        ScheduledDays.Add(key, value);
    }
}

Answer 1

As far as your question is concerned, I'm not sure either choice is best, but if you had to choose one or the other, I would let them overwrite an existing one.

Consider that right now the user has no way to override a workday with a new one directly, so they would have to call GetWorkday to get a reference to it, and then change the properties of that object.

I think the best option would be to provide an overload that takes a Boolean that allows the user to determine if they want to overwrite a value:

public void SetWorkday( Workday value, bool overwrite)

Also, you haven't provided them with any way to RemoveWorkday, which might be a helpful addition.

---

For code reviewing thoughts, what are the fields DayStart and DayEnd for? I thought a workday was just a single day? Seems like it should just have a Date field.

---

In your get workday method, you could add a break; when you find the day you're looking for (since you don't need to keep searching):

foreach( KeyValuePair<DateTime, Workday> pair in ScheduledDays)
{
    if( pair.Key.Date == date.Date)
    {
        result = pair.Value;
        break;
    }
}

---

In your SetWorkday method, I think you only need to pass in a Workday object, not a DateTime key. The reason is that a Workday already has a DateTime associated with it, and looking at your code, it seems like it's an easy entry point for a bug where someone passes in a key whose date is different than Workday.DayStart:

foreach( KeyValuePair<DateTime, Workday> pair in ScheduledDays)
{
    if( pair.Key.Date == value.DayStart.Date)  // <- Comparing the workday date value
    {
        ScheduledDays[key] = value;            // <- But setting it using the key value
        scheduled = true;
        break;
    }
}

Also, in a few places you are checking if a key exists, and if it does you use assign the value using the index, but if it doesn't then you call the Add method. This is unnecessary - you can use the index assignment syntax for both. This will simplify your code. Here are 3 examples of different options and the simplified syntax:

// Overwrites
public void SetWorkday(Workday value)
{
    ScheduledDays[value.DayStart.Date] = value;
}

// Allows Overwriting
public void SetWorkday(Workday value, bool overwrite)
{
    if (ScheduleDays.ContainsKey(value.DayStart.Date) && !overwrite)
    {
        throw new WorkdayOverwriteException();
    }

    ScheduledDays[value.DayStart.Date] = value;
}

// Does not allow overwriting
public void SetWorkday(Workday value)
{
    if (ScheduleDays.ContainsKey(value.DayStart.Date))
    {
        throw new WorkdayOverwriteException();
    }

    ScheduledDays[value.DayStart.Date] = value;
}