Coding style

[Sergey Zuyev]

Hello All
I work at software company and we are having a really big discussion about
coding styles and it seems that more people
prefer statement 1 to statement2 , which was a big surprise to me. Please
help us with your comments. Thanks

Which way is better way 1 or 2?

1. 'set enable status on CheckboxPrimaryYN

If (Me.CheckBoxPrimaryYN.Checked = True) And (dr.PrimarySet 0) Then

Me.CheckBoxPrimaryYN.Enabled = True

Else

If (Me.CheckBoxPrimaryYN.Checked = False) Then

If (dr.PrimarySet = 0) Then

Me.CheckBoxPrimaryYN.Enabled = True

Else

Me.CheckBoxPrimaryYN.Enabled = False

End If

End If

End If

2. 'set enable status on CheckboxPrimaryYN

Me.CheckBoxPrimaryYN.Enabled = Not (Me.CheckBoxPrimaryYN.Checked =
False And dr.PrimarySet = 1)

--
Programmer

[Greg]

"Herfried K. Wagner [MVP]" <hi***************@gmx.atwrote in message
news:eS**************@TK2MSFTNGP06.phx.gbl...

"Greg" <Gr**@no-reply.okschrieb:

>CheckBoxPrimaryYN.Enabled = IIf(Not CheckBoxPrimaryYN.Checked And
dr.PrimarySet, False, True)

I'd loose the unnecessary 'IIf' with its additional boxing overhead too,
because 'Not CheckBoxPrimaryNY.Checked AndAlso dr.PrimarySet = 1' already
returns a boolean value.

Thanks Herfried.

However, losing the IIf and using the AndAlso would then require an extra
Not. ie:

CheckBoxPrimaryYN.Enabled = Not (Not CheckBoxPrimaryYN.Checked AndAlso
dr.PrimarySet)

As the OP's question related to which statement is easier to read/maintain,
I'd prefer not to use double negatives.
If I was to pick up someone else's code and saw the code above, I'd have to
use a few brain cells to determine what it was trying to achieve. If,
however, I saw the code that Jonathan provided in an earlier post then I'd
have no trouble understanding what was happening.

So my answer to the OP is that I wouldn't use either of his Statement1 or
Statement2 suggestions. Instead, I'd use Jonathan's suggestion of:

If Not CheckBoxPrimaryYN.Checked And dr.PrimarySet Then
CheckBoxPrimaryYN.Enabled = False
Else
CheckBoxPrimaryYN.Enabled = True
End If

However, as the result of the If statement is only returning a single
action, I'd use the IIf statement.
If multiple actions were required as a result of the If statement, I'd
certainly choose Jonathan's format.

Cheers,
Greg.

[Branco Medeiros]

Hi,

There are some discrepancies in the OP's examples. The conditions
stated in his first one aren't the same as those in the second.

According to the first example, we have the following conditions to
enable the checkbox:

a) It's checked and dr.PrimarySet 0
b) It's unchecked and dr.PrimarySet = 0

And we have this condition to disable the CheckBox:

c) It's unchecked and dr.PrimarySet <0

Notice that nothing is said about what to do if the checkbox is checked
but dr.PrimarySet <= 0. According to the code, we do nothing, that is,
we leave the checkbox.Enabled property in it's current state. Now, it
may be part of the logic of the application or an omission. We have no
way to tell, since we're not informed about the possible values of
dr.PrimarySet.

Consider now the second example. According to it, we Disable the
Checkbox if:

a) It's not Checked and dr.Primary = 1;

otherwise we enable it.

This states a completelly different logic from the 1st example, and
there's no way to compare both, even though they deal with the same
objects. We can only say that the second example has no relation to the
first, or one of them is a completely mistaken representation.

The OP says he prefers the second example, and issues a statement
similar to:

CheckBox.Enabled = Not( Not CheckBox.Checked _
AndAlso dr.PrimarySet = 1)

To simplify this expression eliminating the double negation, we may
take advantage of the fact that:

Not (A And B) -Not(A) Or Not(B)

Using our current expression (where A =CheckBox.Checked, B=>
dr.PrimarySet = 1):

Not(Not(A) And B)
-Not(Not(A)) Or Not(B)
-A Or Not B

Thus:

CheckBox.Enabled = CheckBox.Checked OrElse dr.PrimarySet <1
Now, if we were to simplify the logic of the first example, we should
decide what to do if the checkbox is checked and dr.PrimarySet <= 0.
I'll assume that when the OP stated dr.PrimarySet 0, he actually
wanted to say dr.PrimarySet <0. I will also assume that if the
Checkbox is set but dr.PrimarySet = 0, we must not mess with the
Enabled property. In other words, to set the Enabled property we'll
have to consider it's current value!

These are, thus, our options:

A) CheckBox.Checked And dr.PrimarySet <0 -Enabled = True
B) CheckBox.Checked And dr.PrimarySet = 0 -Enabled = Enabled
C) CheckBox.Unchecked and dr.PrimarySet = 0 -Enabled = True
D) CheckBox.Unchecked And dr.PrimarySet <0 -Enabled = False

If we were to state this logic in a single expression, it could be:

Dim A As Boolean = CheckBox.Checked
Dim B As Boolean = dr.PrimarySet <0
Dim C As Boolean = CheckBox.Enabled

CheckBox.Enabled = Not(A OrElse B) _
OrElse (A AndAlso (B OrElse C))

Arguably, it's much more clear to use an If:

If CheckBox.Checked Then
CheckBox.Enabled = (dr.PrimarySet <0) OrElse CheckBox.Enabled
Else
CheckBox.Enabled = dr.PrimarySet = 0
End If

In any way, let's hope the VB designers introduce a ternary operator in
the language for the next version:

CheckBox.Enabled = CheckBox.Checked? _
(dr.PrimarySet <0) OrElse CheckBox.Enabled | dr.PrimarySet = 0

=))))

Have fun,

Branco

[Herfried K. Wagner [MVP]]

"Greg" <Gr**@no-reply.okschrieb:

However, losing the IIf and using the AndAlso would then require an extra
Not. ie:

CheckBoxPrimaryYN.Enabled = Not (Not CheckBoxPrimaryYN.Checked AndAlso
dr.PrimarySet)

As the OP's question related to which statement is easier to
read/maintain, I'd prefer not to use double negatives.

You could easily get rid of the double negation by applying De Morgan's law:

<URL:http://en.wikipedia.org/wiki/De_Morgan%27s_laws>:

NOT(A AND B) = (NOT A) OR (NOT B)
NOT(A OR B) = (NOT A) AND (NOT B)

=>

\\\
Me.CheckBoxPrimaryNY.Enabled = _
CheckBoxPrimaryYN.Checked OrElse Not dr.PrimarySet

--
M S Herfried K. Wagner
M V P <URL:http://dotnet.mvps.org/>
V B <URL:http://classicvb.org/petition/>