Clever Geek Handbook

C # - Can I clean this up?

This function is called from form_onload. I am basically reading the registry, determining which checkboxes are checked and then reflecting that in the GUI.

Is there a way to condense this and write better code? How do I use the CheckState property?

Thank.

Woody

private void checkExcelSettings()
    {
        // Read what the values are for the checkboxes first and assign them to a string.
        string _excelEnable = Convert.ToString(Registry.GetValue(@"HKEY_CURRENT_USER\Software\Mask Data\", "ExcelEnableHash", "Unchecked"));
        string _excelSSN = Convert.ToString(Registry.GetValue(@"HKEY_CURRENT_USER\Software\Mask Data\", "ExcelSSNHash", "Unchecked"));
        string _excelCC = Convert.ToString(Registry.GetValue(@"HKEY_CURRENT_USER\Software\Mask Data\", "ExcelCCHash", "Unchecked"));
        string _excelWells = Convert.ToString(Registry.GetValue(@"HKEY_CURRENT_USER\Software\Mask Data\", "ExcelWellsHash", "Unchecked"));
        string s=@"t\""; //unimportant no-op to placate stackoverflow syntax highlighter.


        // Now let make sure we reflect what they are supposed to be in the GUI.
        if (_excelEnable == "Checked")
        {
            chkbxExcelEnable.Checked = true;
        }
        else
        {
            chkbxExcelEnable.Checked = false;
        }

        if (_excelSSN == "Checked")
        {
            chkbxExcelSSN.Checked = true;
        }
        else
        {
            chkbxExcelSSN.Checked = false;
        }

        if (_excelCC == "Checked")
        {
            chkbxExcelCC.Checked = true;
        }
        else
        {
            chkbxExcelCC.Checked = false;
        }

        if (_excelWells == "Checked")
        {
            chkbxExcelWellsFargo.Checked = true;
        }
        else
        {
            chkbxExcelWellsFargo.Checked = false;
        }
    }
+2


source to share


6 answers


Ok, you could at least narrow it down to:

chkbxExcelCC.Checked = _excelCC.Equals("Checked");


This way you avoid all if / else statements.

+11


source


You can remove all unnecessary if / else conditions by putting an inline condition with an assignment. You can also make the primary key path constant. However, to really simplify it, you can repeat the logic of the key lookup and comparison with Checked and put it in a separate method:



private void checkExcelSettings()
{
    // Now let make sure we reflect what they are supposed to be in the GUI.
    chkbxExcelEnable.Checked = IsChecked("ExcelEnableHash");
    chkbxExcelSSN.Checked = IsChecked("ExcelSSNHash");
    chkbxExcelCC.Checked = IsChecked("ExcelCCHash");
    chkbxExcelWellsFargo.Checked = IsChecked("ExcelWellsHash");
}

private static bool IsChecked(string regValue)
{
    return Convert.ToString(
               Registry.GetValue(
                   @"HKEY_CURRENT_USER\Software\Mask Data\",
                   regValue,
                   "Unchecked")) == "Checked";
}
+8


source 


private void checkExcelSettings()
{
    // Read what the values are for the checkboxes first and assign them to a string.
    string _excelEnable = Convert.ToString(Registry.GetValue(@"HKEY_CURRENT_USER\Software\Mask Data\", "ExcelEnableHash", "Unchecked"));
    string _excelSSN = Convert.ToString(Registry.GetValue(@"HKEY_CURRENT_USER\Software\Mask Data\", "ExcelSSNHash", "Unchecked"));
    string _excelCC = Convert.ToString(Registry.GetValue(@"HKEY_CURRENT_USER\Software\Mask Data\", "ExcelCCHash", "Unchecked"));
    string _excelWells = Convert.ToString(Registry.GetValue(@"HKEY_CURRENT_USER\Software\Mask Data\", "ExcelWellsHash", "Unchecked"));

    // Now let make sure we reflect what they are supposed to be in the GUI.
    chkbxExcelEnable.Checked = (_excelEnable == "Checked");
    chkbxExcelSSN.Checked = (_excelSSN == "Checked");
    chkbxExcelCC.Checked = (_excelCC == "Checked");
    chkbxExcelWellsFargo.Checked = (_excelWells == "Checked");
}
+3


source


You can put all your checkboxes in the panel and set the Tag property of each checkbox to the corresponding key in the registry. Then you can use this code in the Load event form:

foreach (CheckBox cb in panel1.Controls)
{
    cb.Checked = ((string)Registry.GetValue(RegPath, 
        (string)cb.Tag, "Unchecked") == "Checked");
}

This has the disadvantage that it will not be immediately apparent to a future maintenance programmer, so I would add a detailed comment here aimed at your possible replacement.

You can also drop the tag and just name each CheckBox with any appropriate key, and then use "cb.Name" instead of "(string) cb.Tag". Anyway, nobody likes Hungarian notation.

0


source 


private void checkExcelSettings()
    {
        string key = @"HKEY_CURRENT_USER\Software\Mask Data\";

        // Read what the values are for the checkboxes first and assign them to a string.
        string _excelEnable = Convert.ToString(Registry.GetValue(key, "ExcelEnableHash", "Unchecked"));
        string _excelSSN = Convert.ToString(Registry.GetValue(key, "ExcelSSNHash", "Unchecked"));
        string _excelCC = Convert.ToString(Registry.GetValue(key, "ExcelCCHash", "Unchecked"));
        string _excelWells = Convert.ToString(Registry.GetValue(key, "ExcelWellsHash", "Unchecked"));
        string s=@"t\""; //unimportant no-op to placate stackoverflow syntax highlighter.

        // Now let make sure we reflect what they are supposed to be in the GUI.
        chkbxExcelEnable.Checked = (_excelEnable == "Checked");
        chkbxExcelSSN.Checked = (_excelSSN == "Checked");
        chkbxExcelCC.Checked = (_excelCC == "Checked");
        chkbxExcelWellsFargo.Checked = (_excelWells == "Checked");
    }
0


source


Use short hand notation if else

chkbxExcelEnable.Checked = _excelEnable == "Checked" ? true: false;
-3


source






More articles:


All Articles