Multiple checkboxes C#

Question

Hi I was wondering if my way of checking which combination of checkbox' that are checked are extremly unefficient? I have previously done it like this: Any improving suggestion on how to iterate through checkboxes to find which combination that is checked? //If no heuristics is active if (checkboxvalues.voyageCutCheckBox == 0) if (checkboxvalues.costEffectivenessCheckBox == 0) if (checkboxvalues.accWaitingCheckBox == 0) if (checkboxvalues.ballastCutCheckBox == 0) { } //If only voyageCutCheckBox is active if (checkboxvalues.voyageCutCheckBox == 1) if (checkboxvalues.costEffectivenessCheckBox == 0) if (checkboxvalues.accWaitingCheckBox == 0) if (checkboxvalues.ballastCutCheckBox == 0) { } //If only costEffectivenessCheckBox is active if (checkboxvalues.voyageCutCheckBox == 0) if (checkboxvalues.costEffectivenessCheckBox == 1) if (checkboxvalues.accWaitingCheckBox == 0) if (checkboxvalues.ballastCutCheckBox == 0) { } //If only accWaitingCheckBox is active if (checkboxvalues.voyageCutCheckBox == 0) if (checkboxvalues.costEffectivenessCheckBox == 0) if (checkboxvalues.accWaitingCheckBox == 1) if (checkboxvalues.ballastCutCheckBox == 0) { } //If only accWaitingCheckBox is active // if (checkboxvalues.voyageCutCheckBox == 0) if (checkboxvalues.costEffectivenessCheckBox == 0) if (checkboxvalues.accWaitingCheckBox == 0) if (checkboxvalues.ballastCutCheckBox == 1) { } //If voyageCutCheckBox and costEffectivenessCheckBox is active if (checkboxvalues.voyageCutCheckBox == 1) if (checkboxvalues.costEffectivenessCheckBox == 1) if (checkboxvalues.accWaitingCheckBox == 0) if (checkboxvalues.ballastCutCheckBox == 0) { } //If voyageCutCheckBox and accWaitingCheckBox is active if (checkboxvalues.voyageCutCheckBox == 1) if (checkboxvalues.costEffectivenessCheckBox == 0) if (checkboxvalues.accWaitingCheckBox == 1) if (checkboxvalues.ballastCutCheckBox == 0) { } //If voyageCutCheckBox and ballastCutCheckBox is active if (checkboxvalues.voyageCutCheckBox == 1) if (checkboxvalues.costEffectivenessCheckBox == 0) if (checkboxvalues.accWaitingCheckBox == 0) if (checkboxvalues.ballastCutCheckBox == 1) { } //If costEffectivenessCheckBox and accWaitingCheckBox is active if (checkboxvalues.voyageCutCheckBox == 0) if (checkboxvalues.costEffectivenessCheckBox == 1) if (checkboxvalues.accWaitingCheckBox == 1) if (checkboxvalues.ballastCutCheckBox == 0) { } //If costEffectivenessCheckBox and ballastCutCheckBox is active if (checkboxvalues.voyageCutCheckBox == 0) if (checkboxvalues.costEffectivenessCheckBox == 1) if (checkboxvalues.accWaitingCheckBox == 0) if (checkboxvalues.ballastCutCheckBox == 1) { i } //If accWaitingCheckBox and ballastCutCheckBox is active if (checkboxvalues.voyageCutCheckBox == 0) if (checkboxvalues.costEffectivenessCheckBox == 0) if (checkboxvalues.accWaitingCheckBox == 1) if (checkboxvalues.ballastCutCheckBox == 1) { } //If voyageCutCheckBox costEffectivenessCheckBox and accWaitingCheckBox is active if (checkboxvalues.voyageCutCheckBox == 1) if (checkboxvalues.costEffectivenessCheckBox == 1) if (checkboxvalues.accWaitingCheckBox == 1) if (checkboxvalues.ballastCutCheckBox == 0) { } //If voyageCutCheckBox costEffectivenessCheckBox and ballastCutCheckBox is active if (checkboxvalues.voyageCutCheckBox == 1) if (checkboxvalues.costEffectivenessCheckBox == 1) if (checkboxvalues.accWaitingCheckBox == 0) if (checkboxvalues.ballastCutCheckBox == 1) { } //If voyageCutCheckBox accWaitingCheckBox and ballastCutCheckBox is active if (checkboxvalues.voyageCutCheckBox == 1) if (checkboxvalues.costEffectivenessCheckBox == 0) if (checkboxvalues.accWaitingCheckBox == 1) if (checkboxvalues.ballastCutCheckBox == 1) { } //If voyageCutCheckBox costEffectivenessCheckBox accWaitingCheckBox and ballastCutCheckBox is active if (checkboxvalues.voyageCutCheckBox == 0) if (checkboxvalues.costEffectivenessCheckBox == 1) if (checkboxvalues.accWaitingCheckBox == 1) if (checkboxvalues.ballastCutCheckBox == 1) { } //If voyageCutCheckBox costEffectivenessCheckBox accWaitingCheckBox and ballastCutCheckBox is active if (checkboxvalues.voyageCutCheckBox == 1) if (checkboxvalues.costEffectivenessCheckBox == 1) if (checkboxvalues.accWaitingCheckBox == 1) if (checkboxvalues.ballastCutCheckBox == 1) { }

Sam Hobbs · Answer

theLizard my code speaks for itself. Can you be specific about what you think does not work? Vague comments such as  you can not use an integer to to represent a binary value having a leading zero  is not conducive to discussion. How is that relevant to my code? What part of my code is affected?

theLizard · Answer

Suthish that is correct 011 as an integer is the same as 11 the leading zero's are lost 001 is the same as 1 same as 000001 as an integer is still 1. Sam what you have tested may have worked for you but you have not shown what or how it worked one thing is for sure you can not use an integer to to represent a binary value having a leading zero you cant even use a float or double it must be a string type Unicode :) or otherwise...

Suthish Nair · Answer

Boolean b = false ; string s = (b == false ? 011 : 110 ); ==> o/p is 011 int i = (b == false ? 011 : 110); ==> o/p is 11

Sam Hobbs · Answer

theLizard you are so predictable; I expected you to say that. It is possible; my code works. I tested it.

theLizard · Answer

You can't convert the Boolean values into a a single integer value it must be a string value 0100 as an integer will become 100 thereby losing the preceding zero.

Sam Hobbs · Answer

It depends on what you need to do based on the values. If you can optimize how you determine what checkboxes are checked and combine that with what is done then you have the potential for making things much more efficient. The efficiency of your original method is insignificant for the computer but it is potentially inefficient for you and other developers. It is very appropriate for you to attempt to make your code as efficient for developers as possible. It might be possible to put processing in the event handler for each checkbox control's checked event but since I don't know what you need to do based on the combinations of values I can't suggest anything that combines both checking what is checked and doing what needs to be done based on what is checked. I think theLizard's suggestion is a good start. What you can do is to convert the various boolean values into one integer value then use an array of delegates to call the function corresponding to the combination. The following will show what I mean. Define a delegate as in: delegate void conditional(); Then declare a List of the delegates: List cl = new List (); I assume you already have a function for each combination of boolean values; the following are skeleton samples but you can probbly use what you already have; maybe with a slight modification. void c0() { } void c1() { } void c2() { } void c3() { } void c4() { } void c5() { } void c6() { } void c7() { } void c8() { } void c9() { } void c10() { } void c11() { } void c12() { } void c13() { } void c14() { } void c15() { } Then in your initialization code such as in the form's constructor build a List of functions. cl.Add(c0); cl.Add(c1); cl.Add(c2); cl.Add(c3); cl.Add(c4); cl.Add(c5); cl.Add(c6); cl.Add(c7); cl.Add(c8); cl.Add(c9); cl.Add(c10); cl.Add(c11); cl.Add(c12); cl.Add(c13); cl.Add(c14); cl.Add(c15); Again note that that is done only once during execution of the program. Then each time you need to find which combination that is checked just do something such as: int values = Convert.ToInt32(voyageCutCheckBox.checked); values += Convert.ToInt32(costEffectivenessCheckBox.checked) values += Convert.ToInt32(accWaitingCheckBox.checked) values += Convert.ToInt32(ballastCutCheckBox.checked) cl[values](); And that will be very efficient. Note that I am not sure what order that the checkboxes need to be in (voyageCutCheckBox .... ballastCutCheckBox or ballastCutCheckBox .... voyageCutCheckBox ); you need to figure that out.

Amund · Answer

Thanks for the suggestion. I will make an attempt on that approach

theLizard · Answer

Just a thought concatenate the values of each of the four checkboxes into a string eg 0100 1000 0010 this may need to be done in a couple of steps eg string values = getbinary(checkbox1.value) + getbinary(checkbox2.value) ... string getbinary(bool value) { return(value==true? 1 : 0); } then you can switch the values eg switch(values) { case 1000: //do your stuff break; case 0100: //do your stuff break; case 0010: //do your stuff break; } I don't think there would be any other way. you could also set the check state in a similar way. Good luck.

Manikavelu Velayutham · Answer

In that case above method is not a better option. You should follow some alogrithmic way to implement it. In that way you can avoid the repeated checks. Just try to find an efficient algorithm to implement your requirement.

Amund · Answer

Thanks for quick reply. OK but if i store the values in local variables I still need 16 entries to iterate through all 16 combinations possible with 4 checkboxes? If i had 10 checkboxes I would still need to list all 100 possible combination outcomes? Or am I missing something in your answer? :)

Manikavelu Velayutham · Answer

First Store the values in local variables. int a = checkboxvalues.voyageCutCheckBox int b = checkboxvalues.costEffectivenessCheckBox int c = checkboxvalues.accWaitingCheckBox int d = checkboxvalues.ballastCutCheckBox Finally use the above variables in your condition. In your code each and everytime you are getting the value of checkbox. If you store the values in the local variable value will be reference only once from the check box.