|
web
newsgroups
|
|||||||||||||||||||||||
|
|||||||||||||||||||||||
Coding styleI 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
Show quote
Hide quote
"Sergey Zuyev" <SergeyZu***@discussions.microsoft.com> wrote in message And it doesn't seem fair that you used so much wording for #1 when #1 could news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 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 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 Show quoteHide quote > "Sergey Zuyev" <SergeyZu***@discussions.microsoft.com> wrote in > message news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 > 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) Show quoteHide quote "Sergey Zuyev" <SergeyZu***@discussions.microsoft.com> wrote in message news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 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. Show quoteHide quote "Sergey Zuyev" <SergeyZu***@discussions.microsoft.com> wrote in message news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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
Show quote
Hide quote
"Jonathan Wood" <jw***@softcircuits.com> wrote in message If I understand the question correctly then this code is incorrect.news:OdhjxXOrGHA.4960@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 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 Greg,
Show quoteHide quote >> I've been programming many years now and, in general, I tend to prefer Did you see the OP's compact code?>> 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. "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.
Show quote
Hide quote
"Jonathan Wood" <jw***@softcircuits.com> wrote in message Jonathan,news:OppSh71rGHA.3324@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 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 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 Show quoteHide quote "Sergey Zuyev" <SergeyZu***@discussions.microsoft.com> schreef in bericht news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 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 Show quoteHide quote "Cor Ligthert [MVP]" <notmyfirstn***@planet.nl> schreef in bericht news:u4aOQiOrGHA.4492@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" <SergeyZu***@discussions.microsoft.com> schreef in bericht > news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 > > "Cor Ligthert [MVP]" <notmyfirstn***@planet.nl> schrieb: I don't think this solution is very intuitive because the property is being > 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. > /// 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/> 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. :-) CorShow quoteHide quote "Herfried K. Wagner [MVP]" <hirf-spam-me-here@gmx.at> schreef in bericht news:O8khHVPrGHA.4016@TK2MSFTNGP05.phx.gbl... > "Cor Ligthert [MVP]" <notmyfirstn***@planet.nl> schrieb: >> 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/> Cor,
> Your answer is as most people think. They think that setting a bit is more But your example does both. It is less efficient.> 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. "Cor Ligthert [MVP]" <notmyfirstn***@planet.nl> schrieb: Sorry, but the sample I posted is shorter than yours and less complex. It > 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. 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/> 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 Show quoteHide quote "Herfried K. Wagner [MVP]" <hirf-spam-me-here@gmx.at> schreef in bericht news:%23f$TgkRrGHA.4444@TK2MSFTNGP02.phx.gbl... > "Cor Ligthert [MVP]" <notmyfirstn***@planet.nl> schrieb: >> 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/> My version is slightly more efficient since yours sometimes requires the
property to be set twice. Show quoteHide quote "Cor Ligthert [MVP]" <notmyfirstn***@planet.nl> wrote in message news:uQSBcMPrGHA.1976@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]" <notmyfirstn***@planet.nl> schreef in bericht > news:u4aOQiOrGHA.4492@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" <SergeyZu***@discussions.microsoft.com> schreef in bericht >> news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 >> >> > > 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 Show quoteHide quote "Jonathan Wood" <jw***@softcircuits.com> schreef in bericht news:upRmMYQrGHA.5008@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]" <notmyfirstn***@planet.nl> wrote in message > news:uQSBcMPrGHA.1976@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]" <notmyfirstn***@planet.nl> schreef in bericht >> news:u4aOQiOrGHA.4492@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" <SergeyZu***@discussions.microsoft.com> schreef in >>> bericht news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 >>> >>> >> >> > > 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? Show quoteHide quote "Cor Ligthert [MVP]" <notmyfirstn***@planet.nl> wrote in message news:%23Vfom6QrGHA.4212@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.com> schreef in bericht > news:upRmMYQrGHA.5008@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]" <notmyfirstn***@planet.nl> wrote in message >> news:uQSBcMPrGHA.1976@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]" <notmyfirstn***@planet.nl> schreef in bericht >>> news:u4aOQiOrGHA.4492@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" <SergeyZu***@discussions.microsoft.com> schreef in >>>> bericht news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 >>>> >>>> >>> >>> >> >> > > 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 Show quoteHide quote "Jonathan Wood" <jw***@softcircuits.com> schreef in bericht news:%2388U7FRrGHA.4356@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]" <notmyfirstn***@planet.nl> wrote in message > news:%23Vfom6QrGHA.4212@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.com> schreef in bericht >> news:upRmMYQrGHA.5008@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]" <notmyfirstn***@planet.nl> wrote in message >>> news:uQSBcMPrGHA.1976@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]" <notmyfirstn***@planet.nl> schreef in bericht >>>> news:u4aOQiOrGHA.4492@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" <SergeyZu***@discussions.microsoft.com> schreef in >>>>> bericht news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 >>>>> >>>>> >>>> >>>> >>> >>> >> >> > > 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. Show quoteHide quote "Cor Ligthert [MVP]" <notmyfirstn***@planet.nl> wrote in message news:ursKzeUrGHA.4236@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.com> schreef in bericht > news:%2388U7FRrGHA.4356@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]" <notmyfirstn***@planet.nl> wrote in message >> news:%23Vfom6QrGHA.4212@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.com> schreef in bericht >>> news:upRmMYQrGHA.5008@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]" <notmyfirstn***@planet.nl> wrote in message >>>> news:uQSBcMPrGHA.1976@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]" <notmyfirstn***@planet.nl> schreef in bericht >>>>> news:u4aOQiOrGHA.4492@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" <SergeyZu***@discussions.microsoft.com> schreef in >>>>>> bericht news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 >>>>>> >>>>>> >>>>> >>>>> >>>> >>>> >>> >>> >> >> > > "Jonathan Wood" <jw***@softcircuits.com> wrote in message Honestly, does it really matter? In cases where bit-twiddling really news:uez8X81rGHA.4512@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 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 Mythran,
> Honestly, does it really matter? In cases where bit-twiddling really It must matter as the person I was replying to kept posting that his code > matters, sure, it would matter... was faster, and now you've posted the same thing. > The following is probably the most efficient code...but hey, does it Since my code only read CheckBoxPrimaryYN.Checked once, I would say it is > 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 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
Show quote
Hide quote
"Jonathan Wood" <jw***@softcircuits.com> wrote in message Hi Jonathon,news:et0eGEAsGHA.596@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 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 Greg,
>> If dr.PrimarySet <> 1 Then Please provide a scenario (state of variables tested) where the code above >> 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 ? produces different results than the OP's statement 2. Thanks.
Show quote
Hide quote
"Jonathan Wood" <jw***@softcircuits.com> wrote in message Actually looking at your code, you're not setting the CheckBox's Enabled news:eNC4zAFsGHA.3804@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. property at all which is the whole point of the OP's Statement2 code? Cheers. 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. Show quoteHide quote "Greg" <G***@no-reply.ok> wrote in message news:ea6ird$26p$1@mws-stat-syd.cdn.telstra.com.au... > "Jonathan Wood" <jw***@softcircuits.com> wrote in message > news:eNC4zAFsGHA.3804@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. >
Show quote
Hide quote
"Jonathan Wood" <jw***@softcircuits.com> wrote in message No problems.news:eIheQ$GsGHA.5072@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. 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.
Show quote
Hide quote
"Greg" <G***@no-reply.ok> wrote in message And, obviously, I'd lose the unneccessary "Me."s giving:news:ea6tqv$8pn$1@mws-stat-syd.cdn.telstra.com.au... > "Jonathan Wood" <jw***@softcircuits.com> wrote in message > news:eIheQ$GsGHA.5072@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) CheckBoxPrimaryYN.Enabled = IIf(Not CheckBoxPrimaryYN.Checked And dr.PrimarySet, False, True) Cheers. "Greg" <G***@no-reply.ok> schrieb: I'd loose the unnecessary 'IIf' with its additional boxing overhead too, > CheckBoxPrimaryYN.Enabled = IIf(Not CheckBoxPrimaryYN.Checked And > dr.PrimarySet, False, True) 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/> 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 Show quoteHide quote "Herfried K. Wagner [MVP]" <hirf-spam-me-here@gmx.at> schreef in bericht news:eSDBUsJsGHA.4612@TK2MSFTNGP06.phx.gbl... > "Greg" <G***@no-reply.ok> schrieb: >> 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/> "Herfried K. Wagner [MVP]" <hirf-spam-me-here@gmx.at> wrote in message Thanks Herfried.news:eSDBUsJsGHA.4612@TK2MSFTNGP06.phx.gbl... > "Greg" <G***@no-reply.ok> schrieb: >> 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. 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. "Greg" <G***@no-reply.ok> schrieb: You could easily get rid of the double negation by applying De Morgan's law:> 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. <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/> 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) Show quoteHide quote "Greg" <G***@no-reply.ok> wrote in message news:ea6tqv$8pn$1@mws-stat-syd.cdn.telstra.com.au... > "Jonathan Wood" <jw***@softcircuits.com> wrote in message > news:eIheQ$GsGHA.5072@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. > > "Jonathan Wood" <jw***@softcircuits.com> schrieb: .... where I'd use:> 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 \\\ 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/> 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. Show quoteHide quote "Herfried K. Wagner [MVP]" <hirf-spam-me-here@gmx.at> wrote in message news:uLWyfrJsGHA.1196@TK2MSFTNGP04.phx.gbl... > "Jonathan Wood" <jw***@softcircuits.com> schrieb: >> 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/> 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 > Okay, I stand corrected. Somewhere along the line, I was mistakenly I still prefer the single line version. Since I'm currently reading Code > 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 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 I'm not sure I see the benefits of moving it to its own function. And
definitely less efficient. Show quoteHide quote "Jim Wooley" <jimNOSPAMwooley@hotmail.com> wrote in message news:24f81e8f781a8c87eb08db27af1@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 > > Cor Ligthert [MVP] wrote:
<snip> > \\\ <snip>> Me.CheckBoxPrimaryYN.Enabled = True > If Me.CheckBoxPrimaryYN.Checked = False AndAlso dr.PrimarySet <> 0 then > me.CheckBoxPrimaryYN.Enabled = False > End if. > /// 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. 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 Show quoteHide quote "Branco Medeiros" <branco.medei***@gmail.com> schreef in bericht news:1153517093.716384.45470@75g2000cwc.googlegroups.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. > 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 Show quoteHide quote > 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) > GhostInAK,
> 1. When testing for equality within an assignment, always place the Ack. FWIW, I hate this approach. This style seems to have originated by > 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.) 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 This is a matter of taste also.> 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. > Comments on your #1 snippet: Ack, cough!> > 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 ...) "Jonathan Wood" <jw***@softcircuits.com> schrieb: ACK, especially with today's IDEs and syntax highlighting it should be easy >> 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?" 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/> 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. Show quoteHide quote "Herfried K. Wagner [MVP]" <hirf-spam-me-here@gmx.at> wrote in message news:%23ysthmRrGHA.4632@TK2MSFTNGP05.phx.gbl... > "Jonathan Wood" <jw***@softcircuits.com> schrieb: >>> 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/> GhostInAK wrote:
<snip> > 1. When testing for equality within an assignment, always place the constant <snip>> 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.) 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.
Show quote
Hide quote
"Sergey Zuyev" <SergeyZu***@discussions.microsoft.com> schrieb: I'd use a simplified version of the the latter solution:> 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) \\\ 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/> Herfried,
I saw that one as well while writing my sample but did not want to go in another direction. :-) CorShow quoteHide quote "Herfried K. Wagner [MVP]" <hirf-spam-me-here@gmx.at> schreef in bericht news:uy5JsPPrGHA.4944@TK2MSFTNGP04.phx.gbl... > "Sergey Zuyev" <SergeyZu***@discussions.microsoft.com> schrieb: >> 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/> > 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. Sergey Zuyev wrote:
Show quoteHide quote > Hello All Is it me or aren't statements 1 and 2 not equal.> 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) > 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
Show quote
Hide quote
"C-Services Holland b.v." <c**@REMOVEcsh4u.nl> wrote in message No it's not you. Statements 1 and 2 are *definitely* different.news:0sOdnT6-bNTlKVnZRVny3w@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 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.
Show quote
Hide quote
"Sergey Zuyev" <SergeyZu***@discussions.microsoft.com> wrote in message G'day. Statement1 and Statement2 do not do the same thing. Statement2 is a news:FBCFC5BE-9706-4BFB-A6BC-2D5A95B54B0F@microsoft.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 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 Sergey Zuyev wrote:
Show quoteHide quote > Hello All 1 is clearer, but harder to read and see where it "fits in". 2 is more> 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 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. 
Other interesting topics
acceptable rounding practices in banking.
Can't get <ToolboxBitmap> to work! Inserting a space into a string Msgbox to asp client? Can I map the data received from a socket directly to a structure in VB.NET Type.TypeOf doesn't work ? Program flow with multi-threading app - corrupted datatables DATAGRID DROPDOWN PROBLEM stop the painting of a form spurious error |
|||||||||||||||||||||||
