How to reduce this IF-Else ladder in c#

Question

This is the IF -Else ladder which I have created to focus first visible control on my form.According to the requirement any control can be hidden on the form.So i had to find first visible control and focus it.

 if (ddlTranscriptionMethod.Visible)
    {
        ddlTranscriptionMethod.Focus();
    }
    else if (ddlSpeechRecognition.Visible)
    {
        ddlSpeechRecognition.Focus();
    }
    else if (!SliderControl1.SliderDisable)
    {
        SliderControl1.Focus();
    }
    else if (ddlESignature.Visible)
    {
        ddlESignature.Focus();
    }
    else
    {
        if (tblDistributionMethods.Visible)
        {
            if (chkViaFax.Visible)
            {
                chkViaFax.Focus();
            }
            else if (chkViaInterface.Visible)
            {
                chkViaInterface.Focus();
            }
            else if (chkViaPrint.Visible)
            {
                chkViaPrint.Focus();
            }
            else
            {
                chkViaSelfService.Focus();
            }
        }
    }

Is there any other way of doing this. I thought using LINQ will hog the performance as i have to tranverse the whole page collection. I am deep on page which has masterpages.Please suggest.

Answer 1

I think your tree is good. This certainly looks like a logic tree that can be simplified, and you have a good sense of smell to be suspicious of it. However, it seems to be that the logic tree reflects what you need. The logic really is this convoluted, and this is the conditional framework that C# gives you to handle this situation. I don't think it can be improved.

If you had a simple list of controls that should have the focus, and you wanted to give focus to the first visible control in the list, you could do this:

(From c in ListOfControls
Where c.visible = true
Select c).First.Focus();

But, it appears you have some additional criteria, so that wouldn't work.

Answer 2

Two approaches:

1. Iterate controls and set focus if visible
2. Use TabIndex and set focus to first. Then focus should fall to first visible control

Answer 3

If you're just trying to focus the first visible control on the form, then I would replace the entire ladder with a single loop:

foreach (Control c in Controls)
{
    if (c.Visible)
    {
        c.Focus();
        break;
    }
}

If you need to focus an inner control, use a recursive method:

bool FocusFirst(ControlCollection controls)
{
    foreach (Control c in controls)
    {
        if (c.Visible)
        {
            c.Focus();
            FocusFirst(c.Controls);
            break;
        }
    }
}

Answer 4

You could return after you meet your criteria, for example:

   if (ddlTranscriptionMethod.Visible) 
    { 
        ddlTranscriptionMethod.Focus(); 
        return;
    }

    if (ddlSpeechRecognition.Visible) 
    { 
        ddlSpeechRecognition.Focus(); 
        return;
    } 

etc..

Answer 5

You can iterate controls and set focus if visible. But I would suggest you to use state pattern for better code readability.

Answer 6

All your doing is setting the focus which is client-side functionality. I would personally do this in javascript (using jQuery). ASP.NET controls that aren't set to visible aren't rendered in the HTML, so you could look for the existence of those elements or there might be an easier way if you're just looking for the first visible, enabled control.

Answer 7

What about a jumpto seems like you could use that here.

Answer 8

When the list of items to evaluate is large (and sometimes it's very large), I try to separate the order of evaluation from the conditional logic, something like this:

List<WebControl> wcBasics = new List<WebControl>();
wcBasics.Add(ddlTranscriptionMethod);
wcBasics.Add(ddlSpeechRecognition);
wcBasics.Add(ddlESignature);

List<CheckBox> checks = new List<CheckBox>();
checks.Add(chkViaFax);
checks.Add(chkViaInterface);
checks.Add(chkViaPrint);

private void Focus()
{
    foreach (WebControl c in wcBasics)
        if (c.Visible) {
            c.Focus();
            return;
        }

    if (!tblDistributionMethods.Visible) return;

    foreach (CheckBox chk in checks)
        if (chk.Visible) {
            chk.Focus();
            return;
        }
    }

    chkViaSelfService.Focus();
}

Answer 9

This is a classic state machine. By setting up a machine it can make the code easier to understand and maintain, though it may add to the total lines.

I won't get into the specifics of your code. By determining what state the user is operating in, you can programmatically change the differing values in an understandable specific state fashion. (Pseudo code below)

enum FormStates
{
    Initial_View
    Working_View
    Edit_View
    Shutdown_View
};

{ // Somewhere in code

switch (theCurrentState)
{

    case Initial_View :
        Control1.Enabled = true;
        Control2.Enabled = true;
        theCurrentState = Working_View;
    break;

   case Working_View
      if (string.empty(Contro1.Text) == false)
      {
          Control2.Enabled = false;
          Speachcontrol.Focus();
          theCurrentState = Edit_view;
      }
      else // Control 2 is operational
      {
         Control1.Enabled = false;
         SliderControl.Focus();
      }

    case Edit_View:
       ...
    break;  

   break;
}

By organizing the code in logical steps, it makes it easier to add more states without jeopardizing an huge if/else.

Answer 10

Here is a slightly different take on the problem. First, define an interface to represent controls which may or may not be focused:

public interface IFormControl
{
    bool Focus();
}

Then create an implementation which handles the easy cases:

public class FormControl : IFormControl
{
    private readonly Control _control;

    public FormControl(Control control)
    {
        _control = control;
    }

    public bool Focus()
    {
        if(_control.Visible)
        {
            _control.Focus();
        }

        return _control.Visible;
    }
}

And create another which handles the more difficult cases:

public class DependentFormControl : IFormControl
{
    private readonly Control _control;
    private readonly Func<bool> _prerequisite;

    public DependentFormControl(Control control, Func<bool> prerequisite)
    {
        _control = control;
        _prerequisite = prerequisite;
    }

    public bool Focus()
    {
        var focused = _prerequisite() && _control.Visible;

        if(focused)
        {
            _control.Focus();
        }

        return focused;
    }
}

Then, create an extension method which sets the focus on the first control in a sequence:

public static void FocusFirst(this IEnumerable<IFormControl> formControls)
{
    var focused = false;

    foreach(var formControl in formControls)
    {
        if(formControl.Focus())
        {
            break;
        }
    }
}

And finally, create the set of controls to be focused:

var controls = new FormControl[]
{
    new FormControl(ddlTranscriptionMethod),
    new FormControl(ddlSpeechRecognition),
    new DependentFormControl(SliderControl1, () => !SliderControl1.SliderDisable),
    new FormControl(ddlESignature),
    new DependentFormControl(chkViaFax, () => tblDistributionMethods.Visible),
    new DependentFormControl(chkViaInterface, () => tblDistributionMethods.Visible),
    new DependentFormControl(chkViaPrint, () => tblDistributionMethods.Visible),
    new DependentFormControl(chkViaSelfService, () => tblDistributionMethods.Visible)
};

controls.FocusFirst();

You can create the set of controls when your page loads and just call .FocusFirst() whenever necessary.