Есть ли более простой способ обработки флажков?

В vb.net у меня есть форма с набором из четырех флажков. Каждый флажок означает, что (когда он установлен) пользователь хочет добавить специальную инструкцию к своему заказу. Код выглядит следующим образом:

        If SpecialInstruction1CheckBox.Checked Then
            AddSpecialInstruction(SPECIAL_INSTRUCTION_1_String)
        End If
        If SpecialInstruction2CheckBox.Checked Then
            AddSpecialInstruction(SPECIAL_INSTRUCTION_2_String)
        End If
        If SpecialInstruction3CheckBox.Checked Then
            AddSpecialInstruction(SPECIAL_INSTRUCTION_3_String)
        End If
        If SpecialInstruction4CheckBox.Checked Then
            AddSpecialInstruction(SPECIAL_INSTRUCTION_4_String)
        End If

У меня такое ощущение, что этот код излишне многословен, повторяется и может быть упрощен. Как мне это сделать, или это не так «неправильно», как кажется?


checkbox if-statement performance vb.net multipleselection
person zdwyer    schedule 12.03.2014    source источник
comment
Это checkBoxList или четыре отдельных флажка?   -  person Tim    schedule 12.03.2014
comment
Конечно, она устарела, но эта статья об управляющих массивах может оказаться полезной: msdn.microsoft.com/en-us/library/aa289500(v=vs.71).aspx   -  person Bob Kaufman    schedule 12.03.2014
comment
@Tim Это четыре отдельных флажка.   -  person zdwyer    schedule 12.03.2014
comment
@BobKaufman Спасибо, я проверю.   -  person zdwyer    schedule 12.03.2014
comment
Являются ли инструкции взаимоисключающими или их можно комбинировать? Если они взаимоисключающие, вы можете использовать оператор Select Case, который выйдет после того, как будет найден первый вариант. (Но если это так, вы можете просто использовать переключатель)   -  person Tim    schedule 12.03.2014
comment
@Tim Их можно комбинировать вместе - пользователь может выбрать ни один, любой или все из них. На самом деле я рассматривал возможность использования Case вместо этого, но, поскольку они не исключают друг друга, я не думаю, что это сработает. Это не было бы проблемой, если бы оператор VB Case/Switch допускал отказ (как это происходит в некоторых вариантах C, таких как Java, когда вы исключаете оператор break   -  person zdwyer    schedule 12.03.2014
comment
Операторы If занимают около 3 наносекунд. Вы можете написать еще 50 миллионов из них незаметно для пользователя. Гадать об эффективности кода редко бывает продуктивно, используйте профайлер. Используйте его достаточно часто, и вам больше не придется гадать.   -  person Hans Passant    schedule 12.03.2014
comment
@HansPassant Извините, я хотел сказать, что мне кажется, что код представляет собой шаблонный код, поскольку вам нужно написать относительно большой объем кода для относительно простой задачи, не столько с точки зрения скорости / эффективности во время выполнения.   -  person zdwyer    schedule 13.03.2014

Ответы (2)


arrow_upward
1
arrow_downward

Первая проблема заключается в том, что ваши специальные инструкции не должны храниться в отдельных переменных. Они должны храниться в массиве или каком-либо другом списке. Затем вы можете получить к ним доступ по индексу (например, specialInstructions(1)).

Затем вы можете перебирать флажки по индексу следующим образом:

For i As Integer = 1 to 4
    Dim box As CheckBox = DirectCast(Me.Controls("SpecialInstruction" & i.ToString() & "CheckBox"), CheckBox)
    If box.Checked Then list.Add(specialInstructions(i))
Next

В качестве альтернативы вы можете хранить ссылки на свои флажки в массиве, а затем более легко перебирать их, например:

Dim checkBoxes() As CheckBox = {
    SpecialInstruction1CheckBox,
    SpecialInstruction2CheckBox,
    SpecialInstruction3CheckBox,
    SpecialInstruction4CheckBox}

' ...

For i As Integer = 0 to checkBoxes.Length - 1
    If checkBoxes(i).Checked Then list.Add(specialInstructions(i))
Next

Другим вариантом было бы сохранить специальные инструкции в свойстве Tag каждого флажка, тогда вы могли бы просто получить значение из элемента управления, например так:

For Each i As CheckBox In checkBoxes
    If i.Checked Then list.Add(i.Tag)
Next

Но это имеет смысл только в том случае, если вам не нужно повторно использовать значения этих специальных инструкций где-либо еще в вашем коде.

person Steven Doggart    schedule 12.03.2014
comment
Capitals предполагает, что инструкции на самом деле являются константами, но да, массивы — интересное решение, если это возможно. Конечно, все инструкции должны быть привязаны к флажкам. - person Crono; 12.03.2014
comment
Однако я бы не стал рекомендовать первый вариант. Просмотр элементов управления по имени сделает код короче, но не оптимальным. - person Crono; 12.03.2014
comment
@ Кроно, согласен. Я предпочитаю последний вариант хранения ссылок в массиве. Оптимальная покупка зависит от того, чего человек пытается достичь. - person Steven Doggart; 12.03.2014
comment
Хорошо, учитывая, что ОП беспокоится об эффективности своего фактического кода (что не так уж плохо), я бы сказал, что он должен беспокоиться о том, чтобы сделать его менее производительным. :п - person Crono; 12.03.2014
comment
@Crono Ну, поскольку «неэффективный» на самом деле не слово, я не хотел предполагать, что это может означать :) - person Steven Doggart; 12.03.2014
comment
Ах, здесь должно быть i, а не u! Не знал этого (не мой язык), но теперь обязательно вспомню, спасибо! :) - person Crono; 12.03.2014
comment
Я определенно использовал бы массив для хранения этих постоянных значений (каждое из них содержит строковый литерал, связанный с необязательными инструкциями для заказа, такими как Хранить в холодильнике или Оставить в магазине UPS для самовывоза) , но я не уверен, что мой инструктор (для колледжа) сочтет это нормальным, поскольку массивы (по крайней мере, в vb.net) не содержат постоянных значений. Есть ли способ защитить массив значений от дальнейших изменений после его инициализации? - person zdwyer; 13.03.2014
comment
Кроме того, это именно тот ответ, который я искал - это заняло у меня минуту, но это определенно то, для чего были созданы массивы. Я все еще изучаю структуры данных, поэтому я не совсем понимаю, что происходит в этом коде, но я знаю, что это то, что я имел в виду - группировка похожих элементов. Спасибо! - person zdwyer; 13.03.2014

arrow_upward
0
arrow_downward

На самом деле код не так уж и настолько плох сам по себе. В основном это зависит от того, что именно делает AddSpecialInstruction. В зависимости от ваших особенностей может быть лучше передать ему список строковых инструкций:

Dim list As New List(Of String)

If SpecialInstruction1CheckBox.Checked Then list.Add(SPECIAL_INSTRUCTION_1_String)
If SpecialInstruction2CheckBox.Checked Then list.Add(SPECIAL_INSTRUCTION_2_String)
If SpecialInstruction3CheckBox.Checked Then list.Add(SPECIAL_INSTRUCTIONIfString)
If SpecialInstruction4CheckBox.Checked Then list.Add(SPECIAL_INSTRUCTION_4_String)

AddSpecialInstructions(list)

Поскольку вам также требовалось сокращение кода, я сделал If операторов, занимающих одну строку. В этом тоже помогут более короткие имена переменных.

person Crono    schedule 12.03.2014