5
\$\begingroup\$

I have seven checkboxes on a data maintenance WinForm, which indicate the days of the week that are "valid" for a particular operation to be performed. These "map" to a list of DayOfWeek enum values on the backing domain object; if the value is present in the list, the box should be checked, otherwise not. Any combination of boxes can be checked. In the "bind" method that sets the UI controls to the proper values of the domain object, here's how those checkboxes are set:

chkValidDayMon.Checked = myModel.ValidDaysOfWeek.Contains(DayOfWeek.Monday);
chkValidDayTue.Checked = myModel.ValidDaysOfWeek.Contains(DayOfWeek.Tuesday);
chkValidDayWed.Checked = myModel.ValidDaysOfWeek.Contains(DayOfWeek.Wednesday);
chkValidDayThu.Checked = myModel.ValidDaysOfWeek.Contains(DayOfWeek.Thursday);
chkValidDayFri.Checked = myModel.ValidDaysOfWeek.Contains(DayOfWeek.Friday);
chkValidDaySat.Checked = myModel.ValidDaysOfWeek.Contains(DayOfWeek.Saturday);
chkValidDaySun.Checked = myModel.ValidDaysOfWeek.Contains(DayOfWeek.Sunday);

The code to read the UI controls' state back out to the domain object is similar; if the box is checked, add the corresponding value to a "clean" (initially empty) list and assign it to the object.

This smells. I was thinking of adding a Dictionary containing the checkboxes and keyed to their corresponding DayOfWeek. But, that sounds like a bit much as well (it certainly wouldn't save many LoC). Any other suggestions?

\$\endgroup\$
6
  • \$\begingroup\$ @Leonid - While in general I agree with you, there are exactly seven days in a Gregorian week (and in most other Abrahamic culture calendars), so until the entire world adopts the Javanese five-day week I don't foresee this code having to change (not in a way that having a Dictionary would make more difficult, anyway). I could use Linq to create the Dictionary based on the controls, maybe. \$\endgroup\$
    – KeithS
    Commented Aug 22, 2012 at 16:37
  • \$\begingroup\$ If you do not want to hide information, then you should go with one of the two approaches that you suggested yourself. Either way you will have to write about 7 lines of code. The dictionary approach is probably better for it cannot be re-factored further. Once the code works, you pretty much know that it will not break. So what that it smells a bit? Seven controls do not justify going overboard, so KISS. The only danger really is programmers who will want to re-factor the working code and may introduce bugs. This is why dictionary is a decent solution. Speed is not an issue at all here. \$\endgroup\$
    – Leonid
    Commented Aug 22, 2012 at 18:03
  • \$\begingroup\$ Well, if you make your comment an answer I'll accept it, because the solution I arrived at was to give the checkboxes Tag values for the integer representations of each DayOfWeek, then slurp the Controls (which were already in a GroupBox) into a Dictionary keyed by the Tag. \$\endgroup\$
    – KeithS
    Commented Aug 22, 2012 at 19:13
  • \$\begingroup\$ the Tag is of the type object. Why not stick an enum value rather than an integer in there? \$\endgroup\$
    – Leonid
    Commented Aug 22, 2012 at 20:03
  • \$\begingroup\$ Because the designer assumes anything you enter into the Tag field is a string, so it actually doesn't really matter either way; I still have to parse it. I could use "Sunday", "Monday" etc as well, it works either way. \$\endgroup\$
    – KeithS
    Commented Aug 22, 2012 at 20:27

1 Answer 1

2
\$\begingroup\$

I'd create an extension method that calculates the day by the name of the checkbox.

I'm using dynamic because I don't know the name of your domain class, just replace it with your name.

Also note, I'm using the web version of CheckBox, substitute ID with whatever version you are using.

Here is the class for the extension:

public static class CheckboxExtensions
{
    public static void DetermineStateFromModel(this CheckBox me, dynamic myModel)
    {
        if (!me.ID.Contains("chkValidDay"))
        {
            // Could throw an exception if required.
            return;
        }

        var dayOfWeek = DayOfWeekFromString(me.ID.Replace("chkValidDay", string.Empty));

        me.Checked = myModel.ValidDaysOfWeek.Contains(dayOfWeek);
    }

    private static DayOfWeek DayOfWeekFromString(string dayOfWeek)
    {
        switch (dayOfWeek)
        {
            case "Mon":
                return DayOfWeek.Monday;
            case "Tue":
                return DayOfWeek.Tuesday;
            // ...
            default:
                throw new ApplicationException("Invalid dayOfWeek specified.");

        }
    }
}

Then your calls would be

chkValidDayMon.DetermineStateFromModel(myModel);
chkValidDayTues.DetermineStateFromModel(myModel);
...

Another thing you might look into is to put the CheckBox objects into a list, then you could just iterate it:

dayCheckBoxList.ForEach(checkBox => checkBox.DetermineStateFromModel(myModel));

Another note, Hungarian casing on variables is not really accepted anymore. It clutters up the code, and there are too many variable types to abbreviate them all to 3 letters. Read this. A better naming convention would be mondayIsValidCheckBox. You would have to modify my extension method to use this new convention.

\$\endgroup\$
7
  • \$\begingroup\$ String parsing on the name of a control? That smells worse than individual binding/unbinding. \$\endgroup\$
    – KeithS
    Commented Aug 22, 2012 at 16:20
  • \$\begingroup\$ I agree that Hungarian casing is bad. I do it on UI controls only, mainly out of old habit (and because changing the type of a control laid out in the Designer requires either ripping it out and replacing it, or touching so much code that changing its name becomes rather trivial) \$\endgroup\$
    – KeithS
    Commented Aug 22, 2012 at 16:27
  • 1
    \$\begingroup\$ String parsing on the name of a control falls into a paradigm call Convention over Configuration: en.wikipedia.org/wiki/Convention_over_configuration which I have been using more and more. \$\endgroup\$ Commented Aug 22, 2012 at 17:47
  • 1
    \$\begingroup\$ Its not hiding as long as it is documented and followed. You also have checks in the methods to make sure the convention is being followed, so if somebody doesn't follow it, an exception is thrown. \$\endgroup\$ Commented Aug 22, 2012 at 17:58
  • 2
    \$\begingroup\$ I think it would be better to use Dictionary<string, DayOfWeek> rather than that big case tree. I have not used C# for some time so I am not sure if the hashmap is called Dictionary in .NET :) \$\endgroup\$
    – Aleksandar
    Commented Sep 21, 2012 at 17:32

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.