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);
}

I'd guess that Saturday and Friday are lower and upper ranges for the items you're retrieving. Why are they properties though, and what are all the other days used for? 0.o — Thorarin
– Thorarin, Commented Aug 5, 2009 at 22:22
Also, I'm somewhat puzzled on why someone with your SO track record and reputation would need others to make this better than it is now. No offense intended, I'm just surprised. — Thorarin
– Thorarin, Commented Aug 5, 2009 at 22:27
haha... i think I at the point where I'm satisfied with it for the purposes that I need. Sat to Fri are set because I need to reference them in my view when I post back to the server. — Jon Erickson
– Jon Erickson, Commented Aug 5, 2009 at 22:33

p.campbell · Accepted Answer · 2009-08-05 21:39:27Z

4

Here's a small start:

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

answered Aug 5, 2009 at 21:39

p.campbell

101k70 gold badges264 silver badges326 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

SolutionYogi · Accepted Answer · 2009-08-05 21:59:45Z

4

Why not create a Dictionary to store these worktimes?

private Dictionary<DayOfWeek, WorkTime> _workTimes;

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

edited Aug 5, 2009 at 21:59

answered Aug 5, 2009 at 21:52

SolutionYogi

32.4k12 gold badges73 silver badges79 bronze badges

Comments

Matt Grande · Accepted Answer · 2009-08-05 21:07:27Z

3

You could use DayOfWeek enum.

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

link

edited Aug 5, 2009 at 21:07

Matt Grande

12.2k6 gold badges64 silver badges89 bronze badges

answered Aug 5, 2009 at 21:03

Eugeniu Torica

7,58412 gold badges49 silver badges63 bronze badges

4 Comments

Neil N Over a year ago

just make sure you take into account first day of week and localization ;)

p.campbell Over a year ago

Even better(?): var dayOfWeek = DateTime.Now.DayOfWeek;

Tim Jarvis Over a year ago

Huh? How does this answer the question? The OP is wanting to populate a bunch of properties with working hours keyed by day.

Jon Erickson Over a year ago

I'm aware of the DayOfWeek enum, but I didn't want a bunch of if's to turn into a switch statement =)

Carl Manaster · Accepted Answer · 2009-08-05 21:49:30Z

0

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));

answered Aug 5, 2009 at 21:49

Carl Manaster

40.5k17 gold badges109 silver badges158 bronze badges

Comments

Tim Jarvis · Accepted Answer · 2009-08-05 21:51:08Z

0

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.

answered Aug 5, 2009 at 21:51

Tim Jarvis

18.9k11 gold badges62 silver badges95 bronze badges

3 Comments

Jon Erickson Over a year ago

although it is possible to do this, but I actually want there to be a null reference for days that do NOT have a work time.

Thorarin Over a year ago

Not in Dictionary = no work time? :) If you want though, you can add them with a null value. Keys cannot be null, but values can.

Jon Erickson Over a year ago

yea, but i'm thinking about the syntax in my View (using MVC). and I think with the above it would make the view simpler

Collectives™ on Stack Overflow

C# refactor this messy code!

5 Answers 5

Comments

Comments

4 Comments

Comments

3 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

Comments

4 Comments

Comments

3 Comments

Your Answer

Sign up or log in

Post as a guest

Related