I am beginner and i wanna ask how to make this code more simple and shorter Thanks for all replies

public static bool SelectedSingleAcc(SpecialForms.AccountManager am)
{
    //checkbox1
    if (am.selectedCB1.Checked == true 
        && am.selectedCB2.Checked == false  
        && am.selectedCB3.Checked == false  
        && am.selectedCB4.Checked == false  
        && am.selectedCB5.Checked == false  
        && am.selectedCB6.Checked == false  
        && am.selectedCB7.Checked == false  
        && am.selectedCB8.Checked == false) return true;
    //checkbox2
    if (am.selectedCB1.Checked == false  
        && am.selectedCB2.Checked == true  
        && am.selectedCB3.Checked == false  
        && am.selectedCB4.Checked == false  
        && am.selectedCB5.Checked == false  
        && am.selectedCB6.Checked == false  
        && am.selectedCB7.Checked == false  
        && am.selectedCB8.Checked == false) return true;
    //checkbox3
    if (am.selectedCB1.Checked == false  
        && am.selectedCB2.Checked == false  
        && am.selectedCB3.Checked == true  
        && am.selectedCB4.Checked == false  
        && am.selectedCB5.Checked == false  
        && am.selectedCB6.Checked == false  
        && am.selectedCB7.Checked == false  
        && am.selectedCB8.Checked == false) return true;
    //checkbox4
    if (am.selectedCB1.Checked == false  
        && am.selectedCB2.Checked == false  
        && am.selectedCB3.Checked == false  
        && am.selectedCB4.Checked == true  
        && am.selectedCB5.Checked == false  
        && am.selectedCB6.Checked == false  
        && am.selectedCB7.Checked == false  
        && am.selectedCB8.Checked == false) return true;
    //checkbox5
    if (am.selectedCB1.Checked == false  
        && am.selectedCB2.Checked == false  
        && am.selectedCB3.Checked == false  
        && am.selectedCB4.Checked == false  
        && am.selectedCB5.Checked == true  
        && am.selectedCB6.Checked == false  
        && am.selectedCB7.Checked == false  
        && am.selectedCB8.Checked == false) return true;
    //checkbox6
    if (am.selectedCB1.Checked == false  
        && am.selectedCB2.Checked == false  
        && am.selectedCB3.Checked == false  
        && am.selectedCB4.Checked == false  
        && am.selectedCB5.Checked == false  
        && am.selectedCB6.Checked == true  
        && am.selectedCB7.Checked == false  
        && am.selectedCB8.Checked == false) return true;
    //checkbox7
    if (am.selectedCB1.Checked == false  
        && am.selectedCB2.Checked == false  
        && am.selectedCB3.Checked == false  
        && am.selectedCB4.Checked == false  
        && am.selectedCB5.Checked == false  
        && am.selectedCB6.Checked == false  
        && am.selectedCB7.Checked == true  
        && am.selectedCB8.Checked == false) return true;
    //checkbox8
    if (am.selectedCB1.Checked == false  
        && am.selectedCB2.Checked == false  
        && am.selectedCB3.Checked == false  
        && am.selectedCB4.Checked == false  
        && am.selectedCB5.Checked == false  
        && am.selectedCB6.Checked == false  
        && am.selectedCB7.Checked == false  
        && am.selectedCB8.Checked == true) return true;
            
    return false;
}

CodePudding user response：

// get all values into an array for easier handling:
values = new[] 
{
    am.selectedCB1.Checked,
    am.selectedCB2.Checked,
    am.selectedCB3.Checked,
    am.selectedCB4.Checked,
    am.selectedCB5.Checked,
    am.selectedCB6.Checked,
    am.selectedCB7.Checked,
    am.selectedCB8.Checked,
};

// find out if exactly one is true
return values.Count(val => val == true) == 1;

That said... the correct UI decision would be to use radio buttons instead of check boxes. They already implement this logic for you. And any time you catch yourself numbering your variables, you should think hard about why this should not be an array instead of many single variables.

CodePudding user response：

public static bool SelectedSingleAcc(SpecialForms.AccountManager am)
        {
            int checkedNumbers = 0;
            if (am.selectedCB1.Checked) checkedNumbers  ;
            if (am.selectedCB2.Checked) checkedNumbers  ;
            if (am.selectedCB3.Checked) checkedNumbers  ;
            if (am.selectedCB4.Checked) checkedNumbers  ;
            if (am.selectedCB5.Checked) checkedNumbers  ;
            if (am.selectedCB6.Checked) checkedNumbers  ;
            if (am.selectedCB7.Checked) checkedNumbers  ;
            if (am.selectedCB9.Checked) checkedNumbers  ;
            
            if (checkedNumbers == 1) //only one checkbox selected, we think the result is true
                 return true;
            
            return false;
            
        }

CodePudding user response：

So it looks like you're checking that at most one checkbox is checked the n return true, otherwise return false

Because, in order to even appear on a form, controls have to be added to a Controls collection we can search through it (using LINQ) looking for controls that match a criteria. In this case we'll look through the controls for any that are checkboxes, and counting those whose name starts with "selectedCB" and whose state is checked. We compare the count to 1, and if there is only 1 then the result is true, if there are 0 or 2 the result is false:

return p.Controls
  .OfType<Checkbox>()
  .Count(c => c.Name.StartsWith("selectedCB") && c.Checked) == 1;

• p is the name of the panel/group box that holds the checkboxes.

• If they are straight on the form use this (or the name of the form variable, if this code isn't part of the form that holds the boxes).

• If there are no other checkboxes in the panel/on the form you can remove the c.Name.StartsWith.... The check on name is purely there to stop e.g. your isActiveCheckbox being checked and hence also polluting the count

• If there are other checkboxes also having a name that starts with "selectedCB", it would be simplest to rename them so that only these 8 checkboxes have a name starting with "selectedCB", or put them in their own panel but add a comment if you want a more refined logic eg a Regex that insists the name be selectedCB[1-8]