By using this site, you agree to our updated Privacy Policy and our Terms of Use. Manage your Cookies Settings.
446,386 Members | 1,727 Online
Bytes IT Community
+ Ask a Question
Need help? Post your question and get tips & solutions from a community of 446,386 IT Pros & Developers. It's quick & easy.

Coding style

P: n/a
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
Jul 21 '06 #1
Share this Question
Share on Google+
52 Replies


P: n/a

"Sergey Zuyev" <Se*********@discussions.microsoft.comwrote in message
news:FB**********************************@microsof t.com...
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
And it doesn't seem fair that you used so much wording for #1 when #1 could
also have been written as follows:

If CheckBoxPrimaryYN.Checked AndAlso dr.PrimarySet 0
CheckBoxPrimaryYN.Enabled = True
ElseIf Not CheckBoxPrimaryYN.Checked
If dr.PrimarySet = 0
CheckBoxPrimaryYN.Enabled = True
Else
CheckBoxPrimaryYN.Enabled = False
End If
End If

Or I would have even taken the mix of both to simplify:

#3
If CheckBoxPrimaryYN.Checked AndAlso dr.PrimarySet 0
CheckBoxPrimaryYN.Enabled = True
ElseIf Not CheckBoxPrimaryYN.Checked
CheckBoxPrimaryYN.Enabled = dr.PrimarySet = 0
End If

The reason behind why so many people would prefer #1 over #2 is
simplification. Otherwords, it's easier to follow #1 than it is #2 (because
the if's are broken down and not compacted onto a single line). People can
follow the if's easier and get the big picture rather than having them
compressed onto a single line and have to use more brainpower to expand the
logic.
HTH :)

Mythran
Jul 21 '06 #2

P: n/a
I would prefer #2 with a twist:

Me.CheckBoxPrimaryYN.Enabled = Me.CheckBoxPrimaryYN.Checked or
dr.PrimarySet <1

or if you prefer the other condition for clarity, then:

Me.CheckBoxPrimaryYN.Enabled = Not (Not Me.CheckBoxPrimaryYN.Checked And
dr.PrimarySet = 1)
"Sergey Zuyev" <Se*********@discussions.microsoft.comwrote in message
news:FB**********************************@microsof t.com...
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

Jul 21 '06 #3

P: n/a
I've been programming many years now and, in general, I tend to prefer the
longer expressions because they often improve readability with no cost to
performance. However, here, I actually think your longer expression is too
convoluted. In addition, it checks CheckBoxPrimaryYN.Checked property
several times, which could impact performance.

So the comparison would be more fair if your "longer version" looked
something like this:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

That said, I think I personally would go ahead with your short version. I
find it easier to understand than your long version (although not
necessarily easier to understand than my long version). And it accesses each
variable/property only once, which means that its probably a little more
efficient.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Sergey Zuyev" <Se*********@discussions.microsoft.comwrote in message
news:FB**********************************@microsof t.com...
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
--
Programmer

Jul 21 '06 #4

P: n/a
I would typically prefer the condensed version #2 as well, potentially with
one of the logic extensions mentioned before. The big downside of the first
method (nested if's), other than maintainability is the possibility that
you won't reset the enabled value on one of the else blocks. In your sample
code, I believe the (Me.CheckBoxPrimaryYN.Checked = False) evaluation won't
reset the enabled status if it is not checked. This may be by design, but
I have seen the case often enough that it is simply an oversight when the
direct assignment of option 2 wouldn't have that problem which helps to facilitate
the maintainability and reliability of the code.

One additional thing to consider which may optomize the expression is to
use logical short-circuting (AndAlso/OrElse) rather than the simple logical
evaluation.

Jim Wooley
http://devauthority.com/blogs/jwooley/default.aspx
"Sergey Zuyev" <Se*********@discussions.microsoft.comwrote in
message news:FB**********************************@microsof t.com...
>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
And it doesn't seem fair that you used so much wording for #1 when #1
could also have been written as follows:

If CheckBoxPrimaryYN.Checked AndAlso dr.PrimarySet 0
CheckBoxPrimaryYN.Enabled = True
ElseIf Not CheckBoxPrimaryYN.Checked
If dr.PrimarySet = 0
CheckBoxPrimaryYN.Enabled = True
Else
CheckBoxPrimaryYN.Enabled = False
End If
End If
Or I would have even taken the mix of both to simplify:

#3
If CheckBoxPrimaryYN.Checked AndAlso dr.PrimarySet 0
CheckBoxPrimaryYN.Enabled = True
ElseIf Not CheckBoxPrimaryYN.Checked
CheckBoxPrimaryYN.Enabled = dr.PrimarySet = 0
End If
The reason behind why so many people would prefer #1 over #2 is
simplification. Otherwords, it's easier to follow #1 than it is #2
(because the if's are broken down and not compacted onto a single
line). People can follow the if's easier and get the big picture
rather than having them compressed onto a single line and have to use
more brainpower to expand the logic.

HTH :)

Mythran

Jul 21 '06 #5

P: n/a
Sergey,

I find #2 an expression for a programmer who likes to obfuscate his/her
sources for others.

It has not even any performanse benefit, but probably took more time to
create and debug than straight written code.

Mostly it is the most simple to set in your code as in this sample the
checkbox to enabled to true, and than check if it maybe should be false and
set it like that, that cost as well the less performance.

Don't expect that the user sees it that you set it to enabled is true. Our
eyes are not fast enough for that while it is not even painted in the
method.

Just my opinion.

Cor

"Sergey Zuyev" <Se*********@discussions.microsoft.comschreef in bericht
news:FB**********************************@microsof t.com...
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

Jul 21 '06 #6

P: n/a
Hello Sergey,

#2 is definately preferable. I would make a few minor changes for clarity
though..

1. When testing for equality within an assignment, always place the constant
to the left of the equality test.
(example: If 1 = x Then ... since you can not assignthe value of x to
1 this is obviously an equality test, not an assignment.)

2. Always group your operation in parenthesees. Yes.. we all know order
of operation will take care of it.. but if you group your operations it's
easy to see exactly whats going on at a glance. Also grouping them makes
it clear where the boolean operations are happening.

So.. to re-write #2 using these rules:
Me.CheckBoxPrimaryYN.Enabled = Not ((Me.CheckBoxPrimaryYN.Checked = False)
And (1 = dr.PrimarySet))
Comments on your #1 snippet:

1. Always write your numeric tests like you are reading a number line.
(example: If you want to know if dr.PrimarySet is greater than 0 then
write:
If 0 < dr.PrimarySet Then ...)

-Boo

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)

Jul 21 '06 #7

P: n/a
For those who don't understand what I mean.

\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0 then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///

Cor
"Cor Ligthert [MVP]" <no************@planet.nlschreef in bericht
news:u4**************@TK2MSFTNGP05.phx.gbl...
Sergey,

I find #2 an expression for a programmer who likes to obfuscate his/her
sources for others.

It has not even any performanse benefit, but probably took more time to
create and debug than straight written code.

Mostly it is the most simple to set in your code as in this sample the
checkbox to enabled to true, and than check if it maybe should be false
and set it like that, that cost as well the less performance.

Don't expect that the user sees it that you set it to enabled is true. Our
eyes are not fast enough for that while it is not even painted in the
method.

Just my opinion.

Cor

"Sergey Zuyev" <Se*********@discussions.microsoft.comschreef in bericht
news:FB**********************************@microsof t.com...
>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


Jul 21 '06 #8

P: n/a
"Sergey Zuyev" <Se*********@discussions.microsoft.comschrieb:
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.

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)
I'd use a simplified version of the the latter solution:

\\\
Me.CheckBoxPrimaryYN.Enabled = _
Not Me.CheckBoxPrimaryYN.Checked AndAlso dr.PrimarySet <1
///

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

Jul 21 '06 #9

P: n/a
Herfried,

I saw that one as well while writing my sample but did not want to go in
another direction.

:-)

Cor

"Herfried K. Wagner [MVP]" <hi***************@gmx.atschreef in bericht
news:uy**************@TK2MSFTNGP04.phx.gbl...
"Sergey Zuyev" <Se*********@discussions.microsoft.comschrieb:
>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.

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)

I'd use a simplified version of the the latter solution:

\\\
Me.CheckBoxPrimaryYN.Enabled = _
Not Me.CheckBoxPrimaryYN.Checked AndAlso dr.PrimarySet <1
///

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

Jul 21 '06 #10

P: n/a
"Cor Ligthert [MVP]" <no************@planet.nlschrieb:
For those who don't understand what I mean.

\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0 then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///
I don't think this solution is very intuitive because the property is being
set to a certain value twice, overwriting the previous value, which
unnecessarily increases the complexity of the code.

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

Jul 21 '06 #11

P: n/a
Herfried,

Your answer is as most people think. They think that setting a bit is more
work than first testing it. That is as we humans do it, therefore are our
brains.

A computer is not human. Setting a bit is less work than testing a bit. I
know that it is really difficult to think like that for a human.

But see my reply on your sample.

:-)

Cor

"Herfried K. Wagner [MVP]" <hi***************@gmx.atschreef in bericht
news:O8**************@TK2MSFTNGP05.phx.gbl...
"Cor Ligthert [MVP]" <no************@planet.nlschrieb:
>For those who don't understand what I mean.

\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0 then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///

I don't think this solution is very intuitive because the property is
being set to a certain value twice, overwriting the previous value, which
unnecessarily increases the complexity of the code.

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

Jul 21 '06 #12

P: n/a
Which way is better way 1 or 2?

I like 2 with comments added. Generally, I prefer shorter source constructs
over longer ones, but if the code is obscure or devious, I want comments that
describe intent.

Jul 21 '06 #13

P: n/a
GhostInAK,
1. When testing for equality within an assignment, always place the
constant to the left of the equality test.
(example: If 1 = x Then ... since you can not assignthe value of x to 1
this is obviously an equality test, not an assignment.)
Ack. FWIW, I hate this approach. This style seems to have originated by
folks concerned about doing an assignment when a compare was intended. But
it's completely unintuitive. You don't ask "is red the color of the barn?"
You ask "Is the barn red?"

That's fine if you prefer this approach (and you're not working on any of my
projects <g>). But as general advice, given without any explanation, I feel
this item is out of place.

Just my opinion.
2. Always group your operation in parenthesees. Yes.. we all know order
of operation will take care of it.. but if you group your operations it's
easy to see exactly whats going on at a glance. Also grouping them makes
it clear where the boolean operations are happening.
This is a matter of taste also.
Comments on your #1 snippet:

1. Always write your numeric tests like you are reading a number line.
(example: If you want to know if dr.PrimarySet is greater than 0 then
write:
If 0 < dr.PrimarySet Then ...)
Ack, cough!

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com
Jul 21 '06 #14

P: n/a
My version is slightly more efficient since yours sometimes requires the
property to be set twice.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Cor Ligthert [MVP]" <no************@planet.nlwrote in message
news:uQ**************@TK2MSFTNGP04.phx.gbl...
For those who don't understand what I mean.

\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0 then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///

Cor
"Cor Ligthert [MVP]" <no************@planet.nlschreef in bericht
news:u4**************@TK2MSFTNGP05.phx.gbl...
>Sergey,

I find #2 an expression for a programmer who likes to obfuscate his/her
sources for others.

It has not even any performanse benefit, but probably took more time to
create and debug than straight written code.

Mostly it is the most simple to set in your code as in this sample the
checkbox to enabled to true, and than check if it maybe should be false
and set it like that, that cost as well the less performance.

Don't expect that the user sees it that you set it to enabled is true.
Our eyes are not fast enough for that while it is not even painted in the
method.

Just my opinion.

Cor

"Sergey Zuyev" <Se*********@discussions.microsoft.comschreef in bericht
news:FB**********************************@microso ft.com...
>>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



Jul 21 '06 #15

P: n/a
Cor,
Your answer is as most people think. They think that setting a bit is more
work than first testing it. That is as we humans do it, therefore are our
brains.

A computer is not human. Setting a bit is less work than testing a bit. I
know that it is really difficult to think like that for a human.
But your example does both. It is less efficient.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com
Jul 21 '06 #16

P: n/a

Cor Ligthert [MVP] wrote:
<snip>
\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0 then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///
<snip>

Your code assumes that setting CheckBoxPrimaryYN.Enabled has no side
effects.

Actually, it probably involves much more than setting a bit: It
probably will, at least, invalidade the area where the checkbox lies
in.

Besides, if there are any actions associated with the event
CheckBoxPrimaryYN.EnableChanged, more side effects may also trigger,
eventually disrupting the application state.

Regards,

Branco.

Jul 21 '06 #17

P: n/a
Jonathan,

That is the human mind, who suposes that testing first is more efficient
while the testing takes the most time.

This is the most simple sample that you often see
If A <1 then A = 1.

This is much quicker and less code for the computer
A = 1.

Cor

"Jonathan Wood" <jw***@softcircuits.comschreef in bericht
news:up**************@TK2MSFTNGP05.phx.gbl...
My version is slightly more efficient since yours sometimes requires the
property to be set twice.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Cor Ligthert [MVP]" <no************@planet.nlwrote in message
news:uQ**************@TK2MSFTNGP04.phx.gbl...
>For those who don't understand what I mean.

\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0 then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///

Cor
"Cor Ligthert [MVP]" <no************@planet.nlschreef in bericht
news:u4**************@TK2MSFTNGP05.phx.gbl...
>>Sergey,

I find #2 an expression for a programmer who likes to obfuscate his/her
sources for others.

It has not even any performanse benefit, but probably took more time to
create and debug than straight written code.

Mostly it is the most simple to set in your code as in this sample the
checkbox to enabled to true, and than check if it maybe should be false
and set it like that, that cost as well the less performance.

Don't expect that the user sees it that you set it to enabled is true.
Our eyes are not fast enough for that while it is not even painted in
the method.

Just my opinion.

Cor

"Sergey Zuyev" <Se*********@discussions.microsoft.comschreef in
bericht news:FB**********************************@microsof t.com...
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




Jul 21 '06 #18

P: n/a
GhostInAK wrote:
<snip>
1. When testing for equality within an assignment, always place the constant
to the left of the equality test.
(example: If 1 = x Then ... since you can not assignthe value of x to
1 this is obviously an equality test, not an assignment.)
<snip>

The assignment operator in VB doesn't act like an expression, so
there's no chance you mistake an equality test with an assignment.

Your suggestion would be more sound if VB performed like the C language
(and similars). In C, the equality operator is '==', so

C == 0

is a comparison, while

C = 0

is an assignment.

More over, in C you can treat comparisons as statements and assignments
as expressions. Therefore,

C == 0;

is a valid statement in C (albeit a harmless one), and

if (C=0) break;

uses an (syntactically correct) assignment as an expression (it assigns
0 to C, tests the result -- which will be 0 -- and breaks if the result
is true -- which it isn't).

As it can be seen, in the C planet it's very easy for the programmer to
write an assignment when he/she meant a comparision, with absolutelly
no help from the compiler to detect the mistake. So the idiom of
placing the constant before the comparision operator is more like a
safeguard. A really advisable one.

This has *zero* chances of happening is VB, the semantics are quite
different. Therefore, this idiom is literally alien to VB, and will
bring no benefit whatsoever other than annoying a few code-readers
(yours trully included).

Regards,

Branco.

Jul 21 '06 #19

P: n/a
You don't appear to be following. Yes, the first one is slower because it
must perform BOTH a comparison and an assignment. But your sample does both.
You have potentially two comparisons and potentially two assignments. The
code I presented has potentially two comparisons but only one assignment.
That's less, and so it's more efficient. Why would you think your version is
more efficient?

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Cor Ligthert [MVP]" <no************@planet.nlwrote in message
news:%2****************@TK2MSFTNGP02.phx.gbl...
Jonathan,

That is the human mind, who suposes that testing first is more efficient
while the testing takes the most time.

This is the most simple sample that you often see
If A <1 then A = 1.

This is much quicker and less code for the computer
A = 1.

Cor

"Jonathan Wood" <jw***@softcircuits.comschreef in bericht
news:up**************@TK2MSFTNGP05.phx.gbl...
>My version is slightly more efficient since yours sometimes requires the
property to be set twice.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Cor Ligthert [MVP]" <no************@planet.nlwrote in message
news:uQ**************@TK2MSFTNGP04.phx.gbl...
>>For those who don't understand what I mean.

\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0 then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///

Cor
"Cor Ligthert [MVP]" <no************@planet.nlschreef in bericht
news:u4**************@TK2MSFTNGP05.phx.gbl...
Sergey,

I find #2 an expression for a programmer who likes to obfuscate his/her
sources for others.

It has not even any performanse benefit, but probably took more time to
create and debug than straight written code.

Mostly it is the most simple to set in your code as in this sample the
checkbox to enabled to true, and than check if it maybe should be false
and set it like that, that cost as well the less performance.

Don't expect that the user sees it that you set it to enabled is true.
Our eyes are not fast enough for that while it is not even painted in
the method.

Just my opinion.

Cor

"Sergey Zuyev" <Se*********@discussions.microsoft.comschreef in
bericht news:FB**********************************@microsof t.com...
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




Jul 21 '06 #20

P: n/a
"Cor Ligthert [MVP]" <no************@planet.nlschrieb:
Your answer is as most people think. They think that setting a bit is more
work than first testing it. That is as we humans do it, therefore are our
brains.
Sorry, but the sample I posted is shorter than yours and less complex. It
depicts the way I am thinking, so I think it's perfect.
A computer is not human. Setting a bit is less work than testing a bit.
I am wondering where you are testing fewer bits than I do in my sample. The
only difference I am able to see is that you are making an unnecessary
assignment.

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

Jul 21 '06 #21

P: n/a
"Jonathan Wood" <jw***@softcircuits.comschrieb:
>1. When testing for equality within an assignment, always place the
constant to the left of the equality test.
(example: If 1 = x Then ... since you can not assignthe value of x to
1 this is obviously an equality test, not an assignment.)

Ack. FWIW, I hate this approach. This style seems to have originated by
folks concerned about doing an assignment when a compare was intended. But
it's completely unintuitive. You don't ask "is red the color of the barn?"
You ask "Is the barn red?"
ACK, especially with today's IDEs and syntax highlighting it should be easy
to distinguish between comparisons and assignments. Personally I do not
even need syntax highlighting to be able to determine if a '=' is an
assignment or a comparison operator in VB.NET.

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

Jul 21 '06 #22

P: n/a
Well, I do know some big companies that require programmers code in that
style. As mentioned elsewhere in this thread, the main reason seems to be
with C, where you could type = instead of == and, with a constant on the
left, the compiler would report an error.

But even that's very iffy (I don't code C that way), and there certainly
doesn't appear to be much reason for it with VB.

In the end, it's a matter of tast. But you don't just tell someone they
should be doing it that way without explanation as though that's the only
way to do it.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Herfried K. Wagner [MVP]" <hi***************@gmx.atwrote in message
news:%2****************@TK2MSFTNGP05.phx.gbl...
"Jonathan Wood" <jw***@softcircuits.comschrieb:
>>1. When testing for equality within an assignment, always place the
constant to the left of the equality test.
(example: If 1 = x Then ... since you can not assignthe value of x to
1 this is obviously an equality test, not an assignment.)

Ack. FWIW, I hate this approach. This style seems to have originated by
folks concerned about doing an assignment when a compare was intended.
But it's completely unintuitive. You don't ask "is red the color of the
barn?" You ask "Is the barn red?"

ACK, especially with today's IDEs and syntax highlighting it should be
easy to distinguish between comparisons and assignments. Personally I do
not even need syntax highlighting to be able to determine if a '=' is an
assignment or a comparison operator in VB.NET.

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

Jul 21 '06 #23

P: n/a
Branco

That is true and in that case you can use a dummy to set it. By the way I
showed this to show how you can do complex "if". I would use in this case
actual the way as is showed by Herfried, in that everybody can in one view
see what is happening.

Cor

"Branco Medeiros" <br*************@gmail.comschreef in bericht
news:11*********************@75g2000cwc.googlegrou ps.com...
>
Cor Ligthert [MVP] wrote:
<snip>
>\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0 then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///
<snip>

Your code assumes that setting CheckBoxPrimaryYN.Enabled has no side
effects.

Actually, it probably involves much more than setting a bit: It
probably will, at least, invalidade the area where the checkbox lies
in.

Besides, if there are any actions associated with the event
CheckBoxPrimaryYN.EnableChanged, more side effects may also trigger,
eventually disrupting the application state.

Regards,

Branco.

Jul 22 '06 #24

P: n/a
Jonathan,

My method shows an alternative for where there are two long comparions
needed. In this actual case is the code as Herfried shows is in my opinion
the best

Cor

"Jonathan Wood" <jw***@softcircuits.comschreef in bericht
news:%2****************@TK2MSFTNGP02.phx.gbl...
You don't appear to be following. Yes, the first one is slower because it
must perform BOTH a comparison and an assignment. But your sample does
both. You have potentially two comparisons and potentially two
assignments. The code I presented has potentially two comparisons but only
one assignment. That's less, and so it's more efficient. Why would you
think your version is more efficient?

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Cor Ligthert [MVP]" <no************@planet.nlwrote in message
news:%2****************@TK2MSFTNGP02.phx.gbl...
>Jonathan,

That is the human mind, who suposes that testing first is more efficient
while the testing takes the most time.

This is the most simple sample that you often see
If A <1 then A = 1.

This is much quicker and less code for the computer
A = 1.

Cor

"Jonathan Wood" <jw***@softcircuits.comschreef in bericht
news:up**************@TK2MSFTNGP05.phx.gbl...
>>My version is slightly more efficient since yours sometimes requires the
property to be set twice.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Cor Ligthert [MVP]" <no************@planet.nlwrote in message
news:uQ**************@TK2MSFTNGP04.phx.gbl...
For those who don't understand what I mean.

\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0 then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///

Cor
"Cor Ligthert [MVP]" <no************@planet.nlschreef in bericht
news:u4**************@TK2MSFTNGP05.phx.gbl...
Sergey,
>
I find #2 an expression for a programmer who likes to obfuscate
his/her sources for others.
>
It has not even any performanse benefit, but probably took more time
to create and debug than straight written code.
>
Mostly it is the most simple to set in your code as in this sample the
checkbox to enabled to true, and than check if it maybe should be
false and set it like that, that cost as well the less performance.
>
Don't expect that the user sees it that you set it to enabled is true.
Our eyes are not fast enough for that while it is not even painted in
the method.
>
Just my opinion.
>
Cor
>
"Sergey Zuyev" <Se*********@discussions.microsoft.comschreef in
bericht news:FB**********************************@microsof t.com...
>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
>
>




Jul 22 '06 #25

P: n/a
Herfried,

Did I in any way deny that, but that what you had, as I told you 4 hours
before your message, I had your code some minutes earlier than you showed
it.

I did not show it because it did not represent what I wanted to show:
Statements should be understandable in one view when reviewing code (not in
this particular case because the code you showed fullfills that).

In this case I would use the code as you have showed myself too.
But if it comes more complex I would not do that.

(Mixing up by instance logical and boolean "Or" or whatever in one
sentence).

Cor

"Herfried K. Wagner [MVP]" <hi***************@gmx.atschreef in bericht
news:%2****************@TK2MSFTNGP02.phx.gbl...
"Cor Ligthert [MVP]" <no************@planet.nlschrieb:
>Your answer is as most people think. They think that setting a bit is
more work than first testing it. That is as we humans do it, therefore
are our brains.

Sorry, but the sample I posted is shorter than yours and less complex. It
depicts the way I am thinking, so I think it's perfect.
>A computer is not human. Setting a bit is less work than testing a bit.

I am wondering where you are testing fewer bits than I do in my sample.
The only difference I am able to see is that you are making an unnecessary
assignment.

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


Jul 22 '06 #26

P: n/a
"Jonathan Wood" <jw***@softcircuits.comwrote in message
news:Od**************@TK2MSFTNGP04.phx.gbl...
I've been programming many years now and, in general, I tend to prefer the
longer expressions because they often improve readability with no cost to
performance. However, here, I actually think your longer expression is too
convoluted. In addition, it checks CheckBoxPrimaryYN.Checked property
several times, which could impact performance.

So the comparison would be more fair if your "longer version" looked
something like this:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If
If I understand the question correctly then this code is incorrect.
The original code was to set the enabled property to true if certain
conditions were met.
It tested for 3 conditions. The fourth possible condition was the default
current state.
ie. we can infer that the enabled property prior to the test was set to
false.
In the code above, the following situation will incorrectly set the enabled
value to true:
If Me.CheckBoxPrimaryYN.Checked = True And dr.PrimarySet = 0

Why? The original code tested 3 situations with the fourth being the
default.
The code above tests all four situations rather than only testing the three
required.
This is a comon mistake I find when programmers try to 'shortcut' another
programmer's code.

Cheers,
Greg
Jul 24 '06 #27

P: n/a
Sergey Zuyev wrote:
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)
Is it me or aren't statements 1 and 2 not equal.
statement 1 states that the checkbox is enabled when:
checked=true and primaryset>0
or
checked=false and primaryset=0

Statement 2 states that the checkbox is enabled when:
either of the 2 statements is false so:
either checked=true or primaryset<>1
splitting up:

when checked=true, primaryset doesn't matter ' (false and something =
false) this contradicts statement 1 where if checked=true primaryset
must be 0

when checked=false primaryset must be unequal to 1 ' without knowing
the valuerange of primaryset, this also contradicts statement 1 where
primary set must be 0. x=0 and x<>1 are only the same thing if the
valuerange of x is 0-1.
--
Rinze van Huizen
C-Services Holland b.v
Jul 24 '06 #28

P: n/a
Greg,
>I've been programming many years now and, in general, I tend to prefer
the longer expressions because they often improve readability with no
cost to performance. However, here, I actually think your longer
expression is too convoluted. In addition, it checks
CheckBoxPrimaryYN.Checked property several times, which could impact
performance.

So the comparison would be more fair if your "longer version" looked
something like this:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

If I understand the question correctly then this code is incorrect.
The original code was to set the enabled property to true if certain
conditions were met.
It tested for 3 conditions. The fourth possible condition was the default
current state.
ie. we can infer that the enabled property prior to the test was set to
false.
In the code above, the following situation will incorrectly set the
enabled value to true:
If Me.CheckBoxPrimaryYN.Checked = True And dr.PrimarySet = 0
Why? The original code tested 3 situations with the fourth being the
default.
The code above tests all four situations rather than only testing the
three required.
This is a comon mistake I find when programmers try to 'shortcut' another
programmer's code.
Did you see the OP's compact code?

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

The code I presented duplicates this logic exactly.

Yes, there were a few problems with the initial post, and of them was that
the compact version has slightly different logic than the longer version.
But I made the best of what I had to work with. And I see no errors in what
I presented.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com
Jul 24 '06 #29

P: n/a
Your code was less efficient even though you contended it was more
efficient. Then again, none of your replies seemed to directly respond to
the post you replied to so this entire exchange was rather tedious.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Cor Ligthert [MVP]" <no************@planet.nlwrote in message
news:ur**************@TK2MSFTNGP04.phx.gbl...
Jonathan,

My method shows an alternative for where there are two long comparions
needed. In this actual case is the code as Herfried shows is in my opinion
the best

Cor

"Jonathan Wood" <jw***@softcircuits.comschreef in bericht
news:%2****************@TK2MSFTNGP02.phx.gbl...
>You don't appear to be following. Yes, the first one is slower because it
must perform BOTH a comparison and an assignment. But your sample does
both. You have potentially two comparisons and potentially two
assignments. The code I presented has potentially two comparisons but
only one assignment. That's less, and so it's more efficient. Why would
you think your version is more efficient?

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Cor Ligthert [MVP]" <no************@planet.nlwrote in message
news:%2****************@TK2MSFTNGP02.phx.gbl...
>>Jonathan,

That is the human mind, who suposes that testing first is more efficient
while the testing takes the most time.

This is the most simple sample that you often see
If A <1 then A = 1.

This is much quicker and less code for the computer
A = 1.

Cor

"Jonathan Wood" <jw***@softcircuits.comschreef in bericht
news:up**************@TK2MSFTNGP05.phx.gbl...
My version is slightly more efficient since yours sometimes requires
the property to be set twice.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Cor Ligthert [MVP]" <no************@planet.nlwrote in message
news:uQ**************@TK2MSFTNGP04.phx.gbl...
For those who don't understand what I mean.
>
\\\
Me.CheckBoxPrimaryYN.Enabled = True
If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <0
then
me.CheckBoxPrimaryYN.Enabled = False
End if.
///
>
Cor
>
>
"Cor Ligthert [MVP]" <no************@planet.nlschreef in bericht
news:u4**************@TK2MSFTNGP05.phx.gbl.. .
>Sergey,
>>
>I find #2 an expression for a programmer who likes to obfuscate
>his/her sources for others.
>>
>It has not even any performanse benefit, but probably took more time
>to create and debug than straight written code.
>>
>Mostly it is the most simple to set in your code as in this sample
>the checkbox to enabled to true, and than check if it maybe should be
>false and set it like that, that cost as well the less performance.
>>
>Don't expect that the user sees it that you set it to enabled is
>true. Our eyes are not fast enough for that while it is not even
>painted in the method.
>>
>Just my opinion.
>>
>Cor
>>
>"Sergey Zuyev" <Se*********@discussions.microsoft.comschreef in
>bericht news:FB**********************************@microsof t.com...
>>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
>>
>>
>
>




Jul 24 '06 #30

P: n/a

"Jonathan Wood" <jw***@softcircuits.comwrote in message
news:ue**************@TK2MSFTNGP02.phx.gbl...
Your code was less efficient even though you contended it was more
efficient. Then again, none of your replies seemed to directly respond to
the post you replied to so this entire exchange was rather tedious.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com
Honestly, does it really matter? In cases where bit-twiddling really
matters, sure, it would matter...

The following is probably the most efficient code...but hey, does it matter?

' --------------------------------------------------------------
Dim checked As Boolean = Me.CheckBoxPrimaryYN.Checked
Dim primarySet As Integer = dr.PrimarySet

If primarySet >= 0
CheckBoxPrimaryYN.Enabled = _
(checked AndAlso primarySet 0) _
OrElse (Not checked AndAlso primarySet = 0)
End If
' --------------------------------------------------------------

Anywho, my 2 cents.

Mythran
Jul 24 '06 #31

P: n/a
"C-Services Holland b.v." <cs*@REMOVEcsh4u.nlwrote in message
news:0s********************@zeelandnet.nl...
Sergey Zuyev wrote:
>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)

Is it me or aren't statements 1 and 2 not equal.
statement 1 states that the checkbox is enabled when:
checked=true and primaryset>0
or
checked=false and primaryset=0

Statement 2 states that the checkbox is enabled when:
either of the 2 statements is false so:
either checked=true or primaryset<>1
splitting up:

when checked=true, primaryset doesn't matter ' (false and something =
false) this contradicts statement 1 where if checked=true primaryset must
be 0

when checked=false primaryset must be unequal to 1 ' without knowing the
valuerange of primaryset, this also contradicts statement 1 where primary
set must be 0. x=0 and x<>1 are only the same thing if the valuerange of x
is 0-1.
--
Rinze van Huizen
C-Services Holland b.v
No it's not you. Statements 1 and 2 are *definitely* different.
Statement 2 tests all four different possibilities whereas Statement 1 only
tests for three of the four possibilities suggesting the missing possibility
is the orignigal state of the CheckBox's Enable property.

Cheers,
Greg.
Jul 25 '06 #32

P: n/a
"Jonathan Wood" <jw***@softcircuits.comwrote in message
news:Op**************@TK2MSFTNGP02.phx.gbl...
Greg,
>>I've been programming many years now and, in general, I tend to prefer
the longer expressions because they often improve readability with no
cost to performance. However, here, I actually think your longer
expression is too convoluted. In addition, it checks
CheckBoxPrimaryYN.Checked property several times, which could impact
performance.

So the comparison would be more fair if your "longer version" looked
something like this:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

If I understand the question correctly then this code is incorrect.
The original code was to set the enabled property to true if certain
conditions were met.
It tested for 3 conditions. The fourth possible condition was the default
current state.
ie. we can infer that the enabled property prior to the test was set to
false.
In the code above, the following situation will incorrectly set the
enabled value to true:
If Me.CheckBoxPrimaryYN.Checked = True And dr.PrimarySet = 0
Why? The original code tested 3 situations with the fourth being the
default.
The code above tests all four situations rather than only testing the
three required.
This is a comon mistake I find when programmers try to 'shortcut' another
programmer's code.

Did you see the OP's compact code?

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

The code I presented duplicates this logic exactly.

Yes, there were a few problems with the initial post, and of them was that
the compact version has slightly different logic than the longer version.
But I made the best of what I had to work with. And I see no errors in
what I presented.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com
Jonathan,

Your code is certainly correct as an alternative to the OP's second solution
where all four possiblities are tested.
The problem lies with the OP's first solution which only tests for three of
the four possibilities suggesting the missing possibility is the orignigal
state of the CheckBox's Enable property. I only compared your solution to
the OP's first solution (I didn't look at the OP's second solution as I
assumed his team were arguing about the best/easiest way to write the *same*
test). Apologies for the confusion. Keep up the good work.

Regards,
Greg

Jul 25 '06 #33

P: n/a
"Sergey Zuyev" <Se*********@discussions.microsoft.comwrote in message
news:FB**********************************@microsof t.com...
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
G'day. Statement1 and Statement2 do not do the same thing. Statement2 is a
nice, easy way to test all four possible conditions. However, if you only
want to test three of the four conditions, then you'd need to use
Statement1. There are many ways to re-code your two statements. Here's a
couple I'd use:

'Statement 1 - Testing 3 of 4 possibilities
If Not (Me.CheckBoxPrimaryYN.Checked = True And dr.PrimarySet = 0) Then
If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1 Then
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If
End If

'Statement2 - Testing all 4 possibilities
If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1 Then
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

Cheers,
Greg
Jul 25 '06 #34

P: n/a
Sergey Zuyev wrote:
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
1 is clearer, but harder to read and see where it "fits in". 2 is more
succinct, but not as clear.

Normally, i'd add a line or so for clarity, as it is much easier to
read the code later. However, this seems a bit too bulky. I'd go with
the second add but a decent comment. If there are no comments, 1 is
better.

B.

Jul 25 '06 #35

P: n/a
Mythran,
Honestly, does it really matter? In cases where bit-twiddling really
matters, sure, it would matter...
It must matter as the person I was replying to kept posting that his code
was faster, and now you've posted the same thing.
The following is probably the most efficient code...but hey, does it
matter?
Dim checked As Boolean = Me.CheckBoxPrimaryYN.Checked
Dim primarySet As Integer = dr.PrimarySet

If primarySet >= 0
CheckBoxPrimaryYN.Enabled = _
(checked AndAlso primarySet 0) _
OrElse (Not checked AndAlso primarySet = 0)
End If
Since my code only read CheckBoxPrimaryYN.Checked once, I would say it is
more efficient than reading it once, copying it to a variable and then
reading that variable twice. And since my code only checks dr.PrimarySet
once, I would say it is more efficient than reading it once, copying it to a
variable and then test that variable three times.

Looking at my code again, it appears it could be further optimized. Note
that I am using the logic of the second version provided by the OP. This is
the syntax I would recommend (I believe it is correct):

If dr.PrimarySet <1 Then
Me.CheckBoxPrimaryYN.Checked = True
End If

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com
Jul 25 '06 #36

P: n/a
"Jonathan Wood" <jw***@softcircuits.comwrote in message
news:et*************@TK2MSFTNGP04.phx.gbl...
>
Since my code only read CheckBoxPrimaryYN.Checked once, I would say it is
more efficient than reading it once, copying it to a variable and then
reading that variable twice. And since my code only checks dr.PrimarySet
once, I would say it is more efficient than reading it once, copying it to
a variable and then test that variable three times.

Looking at my code again, it appears it could be further optimized. Note
that I am using the logic of the second version provided by the OP. This
is the syntax I would recommend (I believe it is correct):

If dr.PrimarySet <1 Then
Me.CheckBoxPrimaryYN.Checked = True
End If
Hi Jonathon,
Your latest code doesn't cater for two of the four tests provided by the
OP's code in Statement2 ?

After some thought, I'd rewrite the OP's Statement2 as follows to cover all
four scenarios:

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

Cheers,
Greg

Jul 26 '06 #37

P: n/a
Greg,
>If dr.PrimarySet <1 Then
Me.CheckBoxPrimaryYN.Checked = True
End If

Hi Jonathon,
Your latest code doesn't cater for two of the four tests provided by the
OP's code in Statement2 ?
Please provide a scenario (state of variables tested) where the code above
produces different results than the OP's statement 2.

Thanks.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com
Jul 26 '06 #38

P: n/a
"Jonathan Wood" <jw***@softcircuits.comwrote in message
news:eN**************@TK2MSFTNGP06.phx.gbl...
Greg,
>>If dr.PrimarySet <1 Then
Me.CheckBoxPrimaryYN.Checked = True
End If

Hi Jonathon,
Your latest code doesn't cater for two of the four tests provided by the
OP's code in Statement2 ?

Please provide a scenario (state of variables tested) where the code above
produces different results than the OP's statement 2.

Thanks.
Actually looking at your code, you're not setting the CheckBox's Enabled
property at all which is the whole point of the OP's Statement2 code?

Cheers.
Jul 26 '06 #39

P: n/a
Greg,
>>>If dr.PrimarySet <1 Then
Me.CheckBoxPrimaryYN.Checked = True
End If

Hi Jonathon,
Your latest code doesn't cater for two of the four tests provided by the
OP's code in Statement2 ?

Please provide a scenario (state of variables tested) where the code
above produces different results than the OP's statement 2.

Thanks.

Actually looking at your code, you're not setting the CheckBox's Enabled
property at all which is the whole point of the OP's Statement2 code?
So far, you seem to just be assuming I'm pretty darn stupid.

So I ask again: Please provide a scenario (state of variables tested) where
my code produces a different results than the OP's statement 2.

I'd be more than happy to discuss this further with you. I may just be
smarter than you think I am. But if you're just going to respond again that
it's wrong, then don't bother.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com
Jul 26 '06 #40

P: n/a
Okay, I stand corrected. Somewhere along the line, I was mistakenly thinking
that it was the Checked property that was being set.

Therefore, I'd stand by my original code:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

Thanks.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Greg" <Gr**@no-reply.okwrote in message
news:ea**********@mws-stat-syd.cdn.telstra.com.au...
"Jonathan Wood" <jw***@softcircuits.comwrote in message
news:eN**************@TK2MSFTNGP06.phx.gbl...
>Greg,
>>>If dr.PrimarySet <1 Then
Me.CheckBoxPrimaryYN.Checked = True
End If

Hi Jonathon,
Your latest code doesn't cater for two of the four tests provided by the
OP's code in Statement2 ?

Please provide a scenario (state of variables tested) where the code
above produces different results than the OP's statement 2.

Thanks.

Actually looking at your code, you're not setting the CheckBox's Enabled
property at all which is the whole point of the OP's Statement2 code?

Cheers.

Jul 26 '06 #41

P: n/a
"Jonathan Wood" <jw***@softcircuits.comwrote in message
news:eI**************@TK2MSFTNGP05.phx.gbl...
Okay, I stand corrected. Somewhere along the line, I was mistakenly
thinking that it was the Checked property that was being set.

Therefore, I'd stand by my original code:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

Thanks.
No problems.
Good, eay to read code.
However, as a personal choice, I'd convert it to a single line IIf statement
to use less lines:

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

Cheers.
Jul 26 '06 #42

P: n/a
"Greg" <Gr**@no-reply.okwrote in message
news:ea**********@mws-stat-syd.cdn.telstra.com.au...
"Jonathan Wood" <jw***@softcircuits.comwrote in message
news:eI**************@TK2MSFTNGP05.phx.gbl...
>Okay, I stand corrected. Somewhere along the line, I was mistakenly
thinking that it was the Checked property that was being set.

Therefore, I'd stand by my original code:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

Thanks.

No problems.
Good, eay to read code.
However, as a personal choice, I'd convert it to a single line IIf
statement to use less lines:

Me.CheckBoxPrimaryYN.Enabled = IIf(Not Me.CheckBoxPrimaryYN.Checked And
Me.dr.PrimarySet, False, True)
And, obviously, I'd lose the unneccessary "Me."s giving:

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

Cheers.
Jul 26 '06 #43

P: n/a
"Jonathan Wood" <jw***@softcircuits.comschrieb:
Okay, I stand corrected. Somewhere along the line, I was mistakenly
thinking that it was the Checked property that was being set.

Therefore, I'd stand by my original code:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If
.... where I'd use:

\\\
Me.CheckBoxPrimaryYN.Enabled = _
Me.CheckBoxPrimaryYN.Checked AndAlso dr.PrimarySet <1
///

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

Jul 26 '06 #44

P: n/a
"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.

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

Jul 26 '06 #45

P: n/a
Herfried,

I was thinking to do the answer the same as you,
I don't know why I decided not to do that.
(Maybe that I did not want to show again how I hate the IIF)

I get the idea that beside me nobody noticed your fine solution.

Cor

"Herfried K. Wagner [MVP]" <hi***************@gmx.atschreef in bericht
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.

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

Jul 26 '06 #46

P: n/a
Okay, I stand corrected. Somewhere along the line, I was mistakenly
thinking that it was the Checked property that was being set.

Therefore, I'd stand by my original code:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If
I still prefer the single line version. Since I'm currently reading Code
Complete, I thought I would throw out another option just to stir the pot
how about refactoring the logic into a separate sub and doing the following.
It may be slightly more overhead, but may aid in maintainability.

CheckBoxPrimaryYN.Enabled = CanEnableBox()

Private Function CanEnableBox() as Boolean
Return Not CheckBoxPrimaryNY.Checked AndAlso dr.PrimarySet = 1
End Sub

Let's see if we can stir this discussion some more.

Jim Wooley
http://devauthority.com/blogs/jwooley
Jul 26 '06 #47

P: n/a
Yes, that's pretty efficient. However, other than using up less space being
slightly less readable, I don't think it's faster than my version.

In my old age, I've grown to prefer the most readable code over saving lines
of source code.

One item that could be more efficient though is use of AndAlso. I would
probably change my code to use that instead.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Herfried K. Wagner [MVP]" <hi***************@gmx.atwrote in message
news:uL**************@TK2MSFTNGP04.phx.gbl...
"Jonathan Wood" <jw***@softcircuits.comschrieb:
>Okay, I stand corrected. Somewhere along the line, I was mistakenly
thinking that it was the Checked property that was being set.

Therefore, I'd stand by my original code:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

... where I'd use:

\\\
Me.CheckBoxPrimaryYN.Enabled = _
Me.CheckBoxPrimaryYN.Checked AndAlso dr.PrimarySet <1
///

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

Jul 26 '06 #48

P: n/a
I'm not sure I see the benefits of moving it to its own function. And
definitely less efficient.

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Jim Wooley" <ji*************@hotmail.comwrote in message
news:24*************************@msnews.microsoft. com...
>Okay, I stand corrected. Somewhere along the line, I was mistakenly
thinking that it was the Checked property that was being set.

Therefore, I'd stand by my original code:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

I still prefer the single line version. Since I'm currently reading Code
Complete, I thought I would throw out another option just to stir the pot
how about refactoring the logic into a separate sub and doing the
following. It may be slightly more overhead, but may aid in
maintainability.

CheckBoxPrimaryYN.Enabled = CanEnableBox()

Private Function CanEnableBox() as Boolean
Return Not CheckBoxPrimaryNY.Checked AndAlso dr.PrimarySet = 1
End Sub

Let's see if we can stir this discussion some more.

Jim Wooley
http://devauthority.com/blogs/jwooley


Jul 26 '06 #49

P: n/a
Based on my preferences, I generally avoid IIf. When I started programming,
I used to like one-liners like that. But these days, I have no problem
stretching out the source to make it easier to read and debug. IIf provides
no advantage over a regular If statement other than compacting your source
and making it slightly less readable. In addition, if IIf still involves an
additional call, then it's less efficient as well.

In addition to that, it is not necessary in your code as you could just do
this:

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

--
Jonathan Wood
SoftCircuits Programming
http://www.softcircuits.com

"Greg" <Gr**@no-reply.okwrote in message
news:ea**********@mws-stat-syd.cdn.telstra.com.au...
"Jonathan Wood" <jw***@softcircuits.comwrote in message
news:eI**************@TK2MSFTNGP05.phx.gbl...
>Okay, I stand corrected. Somewhere along the line, I was mistakenly
thinking that it was the Checked property that was being set.

Therefore, I'd stand by my original code:

If Me.CheckBoxPrimaryYN.Checked = False And dr.PrimarySet = 1)
Me.CheckBoxPrimaryYN.Enabled = False
Else
Me.CheckBoxPrimaryYN.Enabled = True
End If

Thanks.

No problems.
Good, eay to read code.
However, as a personal choice, I'd convert it to a single line IIf
statement to use less lines:

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

Cheers.


Jul 26 '06 #50

52 Replies

This discussion thread is closed

Replies have been disabled for this discussion.