Winforms app to create AD users, using configuration dialog to remove options from parent form.

Question

I can't seem to get this to work.

I have a windows app called the user creation tool. I have it set up to create an AD user and it has a checkedlistbox on a second form that I want to let me remove unnecessary options from the first forms checkedlist box. However, I can't seem to make this work.

This is my main form, you go to the AD Groups tab to set group membership using a checkedlistbox seen in the second image.

Then you can hit configure and I want it to let me remove options from the checkedlistbox in the first form. But I can't get it to do it.

]3

Here is the code I am trying that hasn't seemed to work yet. I have the first page set up to save an instance of itself. Then a checkedlistbox object called groupchecklist and the value is set in the main method to the name of the checkedlistbox on this page.

    public static UserCreationTool instance;  
    public CheckedListBox GroupChecklist;  
    public UserCreationTool()  
    {  
        InitializeComponent();  
        RRGetADCreds();  
        RRGetAllGroups();  
        instance = this;  
        GroupChecklist = chklbADGroups;  
    }  

then on my configuration form I am trying this:

    private void WhatIsChecked_Click()  
    {  
        foreach (int indexChecked in chklbDisableGroups.CheckedIndices)  
        {  
            chklbDisableGroups.GetItemCheckState(indexChecked);   
            UserCreationTool.instance.GroupChecklist.Items.Remove(indexChecked);  
        }  

This method is linked to the apply button and should remove anything that is checked in the box on the second form from the first form. However, it does not. Can anyone see where I'm messing this up? Thanks for the help!

Answer 1

In general I don't recommend that you update controls in one "container" from another just because it tightly couples the code together too much. My preference is to make this more dynamic. But let's just go with what you have. Firstly I notice that you're calling GetItemCheckState but this just returns the state of the checkbox and since you are enumerating the checked items this should always return the item is checked so it is a useless call and can be removed.

The second thing I notice is that you are relying on the fact that the index in the second tab matches the index in the first tab and that isn't going to work. Imagine that you have 5 items in your first tab (e.g. "1"..."5"). You have the same thing in the second tab. Your user goes to the second tab and unchecks items "3" and "5". Your code will attempt to remove the same indexed items in the first tab which will work correctly this time. However since you removed them the first tab now only has ("1", "2' and "4"). Now the user goes back to the second tab and unchecks "4". In the second tab this is index 3, I'd guess, but it is index 2 in the first tab (because you removed other items already). Using an index to match up collections/arrays only works if they always have the same items and your code enforces that. Any insertions or deletions from either set will invalidate that.

For your specific case I would recommend that you enumerate through the items in the second tab and for each item you find the corresponding item in the first tab's list (probably using the display text or perhaps the value if you're setting that). If the item is found then you can remove it otherwise it has already been removed.

The third issue I see with your code, and perhaps it is just a misunderstanding of your ultimate goal, is that the user is going to the second tab to check the items to remove from the first tab. But once an item is removed it cannot be added back and therefore having the second tab show all the items probably doesn't make sense so you should remove them from both. Of course if you want the user to be able to add and remove based upon the second list then your logic is probably backwards. You'd like want the checked items to be shown and the unchecked items to be removed. Therefore your configure function should enumerate all the items in the second list and add the option back if the item is checked or remove it if it isn't.

As for the "perfect" solution to me I would forgo keeping the lists in sync altogether and store the list of groups (or whatever you're showing) in a simple collection along with its visibility state. For your configuration function in the second tab you simply toggle the visibility state of the data on and off as needed. In the first tab you render the control items by enumerating the data and only showing those that are set to visible. This means that each time the tab is shown you have to refresh the list of selected items. You can do that by handling the Selected event of the tab control.