Whats the best way to group a lot of controls for passing as parameters to a method?

Question

I have a Windows Form with A LOT of controls separated into groups based on the day (Monday, Tuesday, etc.), and functions dependent on the controls within that day. I want to find a better way to structure my code, because right now it feels and looks like spaghetti.

Currently, my code is technically working, but it's a mess. It's way too long, and when I need to edit some of my functions I'm having to edit them for 7 different days and find/replace control names and stuff.

With the help of a more experienced colleague who suggested using parameters (duh), I've found a way to simplify it some. I haven't applied it to my main code yet, but have played around with it in a simpler test environment.

The problem now is that I have an absurd number of parameters that I'm hoping can be grouped somehow.

I've looked into the GroupBox control, but it doesn't seem to do what I hoped.

Here's a simplified example of code that I'm working with. In my actual code, there are a lot more controls involved, and therefore a lot more parameters.

private void validate(DateTimePicker start, DateTimePicker end, DateTimePicker lunch, Label day)
{
    if ((lunch.Value < start.Value || lunch.Value >= end.Value))
    {
        day.Visible = true;
    }
    if ((lunch.Value >= start.Value && lunch.Value < end.Value))
    {
        day.Visible = false;
    }
}

I have a feeling I should be using an array to group these? Can anyone confirm? Thanks.

TL;DR: How can you group a lot of parameters for passing to a function?

Answer 1

I'm not sure if the small scope you share with us is enough to give a meaningful answer; but neither I'm sure sharing with us a bigger part of the code would be really helpful.

Anyway, with the small information you gave us, I can think of a couple suggestions:

1. Create a custom control.

If you say you have group of controls that repeat themselves once for each day of the week, and each group of control executes the same logic, it would be better to create a custom control that contains both all the controls each day needs, and the logic within each group of controls. Add methods, functions and events to control its behaviour and its interaction with the rest of the form. Then just put as many instances of that control as you need, and let the control control itself.

2. Don't pass controls as parameters, pass their values.

Passing the controls as parameters adds little to no value to your code, increases complexity, tightens coupling and could even be considered a code smell. Instead, create methods and function that receive values as parameters, and call them using the values of the controls. With this, you could even create a POCO that contains all the values needed, and receive that as parameter - just make sure each POCO is related to the logical design and not to the functions instead (i.e. don't create one for each function; create 4 or 5 that can contain all necesary values and receive as many as needed in each function). This would also be a step into the 'decoupling' direction, allowing you to eventually separate your code in layers.

3. Use return values instead of reference parameters. One thing that increases the number of parameters received by your method is that you also receive the object whose state will be modified by it. You could instead return a value and assign it to the target control, both reducing the number of parameters and increasign the usefullness of your function.

Taking these points in consideration, and using your example, instead of this:

private void validate(DateTimePicker start, DateTimePicker end, DateTimePicker lunch, Label day)
{
    if ((lunch.Value < start.Value || lunch.Value >= end.Value))
    {
        day.Visible = true;
    }
    if ((lunch.Value >= start.Value && lunch.Value < end.Value))
    {
        day.Visible = false;
    }
}

You could have this:

private bool validate(DateTime start, DateTime end, DateTime lunch)
{
    return lunch < start || lunch > end;
}

public bool validateControl()
{
    //executes all validations
    this.lblDay.Visible = validate(dtpStart.Value, dtpEnd.Value, dtpLunch.Value);
}