1

I have three checkboxes in my form. The problem is that when I check all three, I only received one checkbox. how can I fix this?

my code:

    private void button1_Click(object sender, EventArgs e)
    {
        ConfigOptions itemToSave = 0;

        if (autoCapsNames.Checked)
        {
            itemToSave |= ConfigOptions.AutoCapsStr;
        }

        if (autoSort.Checked)
        {
            itemToSave |= ConfigOptions.IntantOrganization;
        }

        if (showLinesNumbers.Checked)
        {
            itemToSave |= ConfigOptions.ShowLinesNumber;
        }


        SaveConfigs(itemToSave);


    }

Thanks

Chase Florell
  • 46,378
  • 57
  • 186
  • 376
Jack
  • 16,276
  • 55
  • 159
  • 284

2 Answers2

2

The previous answer should solve your issue, but it changes the way you handle the data. If you want to use bitwise OR method (as you are doing now), make sure you have correctly defined ConfigOptions. Values assigned to ConfigOptions.AutoCapsStr, ConfigOptions.IntantOrganization and ConfigOptions.ShowLinesNumber should be chosen in such a way to define the values you set in a unique way.

If ConfigOptions is an enum, you can try to define it like this:

enum ConfigOptions
{
    AutoCapsStr = 1, 
    IntantOrganization = 2, 
    ShowLinesNumber = 4
}

Then, you can use it inside your SaveConfigs method (or in your loading method, if you just save the numeric value) to test values set like this:

if (itemToSave & ConfigOptions.AutoCapsStr != 0)
{
    //ConfigOptions.AutoCapsStr is set, so do appropriate things here
}
Lukasz M
  • 5,635
  • 2
  • 22
  • 29
  • Thanks for +1 :). Using OR this way is not very intuitive, but can be helpful sometimes. I'm glad the information is useful for You :). – Lukasz M Jan 25 '12 at 18:09
  • 1
    Apparently it's a good idea to use the `FlagsAttribute` on this type of enum. http://stackoverflow.com/a/8480/124069 – Chase Florell Jan 25 '12 at 18:15
  • Also, bit shifting can be useful in this as well, so that you don't have to deal with the calculations. `enum ConfigOptions{AutoCapsStr = 1 << 0, IntantOrganization 1 << 1, ShowLinesNumber 1 << 2}` – Chase Florell Jan 25 '12 at 18:19
0

Your code might only be setting the itemToSave once. If all your checkboxes are checked, you could end up only saving showLineNumbers since you're overwriting itemToSave through every if statement.

You might to want to change your approach a little. You could

  1. SaveConfigs(itemToSave); inside each if statement, thereby saving each config option individually.
  2. Store each itemToSave inside a List<T>, and loop through the list, saving what you need at the end.

Here's a quick and dirty approach to get you started

private void button1_Click(object sender, EventArgs e)
{
    List<ConfigOptions> itemsToSave = default(List<ConfigOptions>);

    if (autoCapsNames.Checked)
    {
        itemsToSave.Add(ConfigOptions.AutoCapsStr);
    }

    if (autoSort.Checked)
    {
        itemsToSave.Add(ConfigOptions.IntantOrganization);
    }

    if (showLinesNumbers.Checked)
    {
        itemsToSave.Add(ConfigOptions.ShowLinesNumber);
    }

    SaveConfigs(itemsToSave); // You're passing the list, and parsing it within the SaveConfigs method.
}

Alternatively if you're going to use int's in a bitwise calculation, please see @Lucas's answer. I've personally never done it that way, but he has a great point. (I've learned something today).

Chase Florell
  • 46,378
  • 57
  • 186
  • 376
  • 1
    He may be not overwriting the itemToSave variable, because he's using |=, not =. It should work if the ConfigOptions is defined correctly. – Lukasz M Jan 25 '12 at 17:41
  • I'm not as well versed in C# as I should be, but doesn't `|=` translate into `itemToSave = itemToSave [OR] newItemToSave`? It appears to me that he wants [AND], yet the `|&` would be used to remove (and not add). /re [Jon Skeet](http://stackoverflow.com/a/6942494/124069) – Chase Florell Jan 25 '12 at 17:49
  • 1
    Yes, it does translate to OR. It's a logical OR, so he cumulatively adds other values by bitwise ORing them i.e.: int i = 0; i|=2; i|=8; gives you i == 10 as a result. You can set selected bits this way and tread reads them to see which options were chosen. – Lukasz M Jan 25 '12 at 17:54
  • 1
    @ChaseFlorell No, Lucas is right. It depends of the defintion of ConfigOptions. – Mr Lister Jan 25 '12 at 17:54
  • 1
    @Lucas I follow you now! That would make a good alternative answer to this question. – Chase Florell Jan 25 '12 at 17:56