C# refactor this messy code!

Question

I am basically creating a flat View Model for a Timesheet page (ASP.NET MVC) that has a grid for the days of the week. The WorkTime properties should either be an existing WorkTime from the database or null if there is no existing one.

There will only ever be 1 week displayed (Saturday to Friday). and I have the individual properties there so that the syntax in my view should be simpler.

+------+-----+-----+-----+-----+-----+-----+-----+
| Desc | Sat | Sun | Mon | Tue | Wed | Thu | Fri |
+------+-----+-----+-----+-----+-----+-----+-----+
|______|_____|_____|_____|_____|_____|_____|_____|
|______|_____|_____|_____|_____|_____|_____|_____|
|______|_____|_____|_____|_____|_____|_____|_____|
|______|_____|_____|_____|_____|_____|_____|_____|

All of the dates that come from and are persisted to the database have no Time element (all midnight hour) and the Saturday to Friday DateTime properties are all set already.

The properties I am setting

public WorkTime SaturdayWorkTime { get; private set; }
public WorkTime SundayWorkTime { get; private set; }
public WorkTime MondayWorkTime { get; private set; }
public WorkTime TuesdayWorkTime { get; private set; }
public WorkTime WednesdayWorkTime { get; private set; }
public WorkTime ThursdayWorkTime { get; private set; }
public WorkTime FridayWorkTime { get; private set; }

Current iteration...

public DateTime Saturday { get; private set; }
public DateTime Sunday { get; private set; }
public DateTime Monday { get; private set; }
public DateTime Tuesday { get; private set; }
public DateTime Wednesday { get; private set; }
public DateTime Thursday { get; private set; }
public DateTime Friday { get; private set; }

_workTimes = _workTimeRepository.GetByWorkAssignmentID(WorkAssignment.ID, Saturday, Friday);
SaturdayWorkTime = GetWorkTimeForDay(DayOfWeek.Saturday);
SundayWorkTime = GetWorkTimeForDay(DayOfWeek.Sunday);
MondayWorkTime = GetWorkTimeForDay(DayOfWeek.Monday);
TuesdayWorkTime = GetWorkTimeForDay(DayOfWeek.Tuesday);
WednesdayWorkTime = GetWorkTimeForDay(DayOfWeek.Wednesday);
ThursdayWorkTime = GetWorkTimeForDay(DayOfWeek.Thursday);
FridayWorkTime = GetWorkTimeForDay(DayOfWeek.Friday);

with this helper method...

private WorkTime GetWorkTimeForDay(DayOfWeek dow)
{
    return _workTimes.FirstOrDefault(x => x.Date.DayOfWeek == dow);
}

Answer 1

You could use DayOfWeek enum.

DateTime now = DateTime.Now;
var dayOfWeek = now.DayOfWeek;

link

Answer 2

That gave me a headache. Really, when it's that bad, start over from scratch.

To all the downvoters: EVERY single answer to this question is a rewrite, is it not? Not one re-used any of the original code.

Answer 3

Here's a small start:

private WorkTime GetWorkTimeForDay(DayOfWeek dw)
{
    return workTimes.FirstOrDefault(x => x.Date.DayOfWeek == dw);
}

Answer 4

Instead of having the individual variables, could you have a map from DayOfWeek to WorkTime, and another map from DayOfWeek to DateTime? Sorry, my C# is rusty, so this probably isn't quite right, but if you had that, then it would look more like this:

for (DayOfWeek day : allDays) 
    workTimes(day) = repository.FirstOrDefault(dateTimes(day));

Answer 5

Why not instead just have one WorkTime property, and make it a Dictionary and return the dictionary (instead of your list) from your worktime repository.

Answer 6

Why not create a Dictionary to store these worktimes?

private Dictionary<DayOfWeek, WorkTime> _workTimes;

Populating this dictionary, using _workTimeRepository.GetByWorkAssignmentID, should be very simple.