|
I vote the latter
|
|
|
|
|
Well, given that they are both VB6...
I can imagine the sinking feeling one would have after ordering my book,
only to find a laughably ridiculous theory with demented logic once the book arrives - Mark McCutcheon
|
|
|
|
|
Andy Brummer wrote: VB6
|
|
|
|
|
I agree that they are both pretty bad. It's the curse of having to dive into some vb6 code on occasion...
|
|
|
|
|
The second lends itself to replacing the values with a file string and thus more flexible
strFileValue = "summer;winter;spring;fall;late fall;early spring;late birthday" 'comes from a configuration file or user settings, or whatever.
dim strArray as string
strArray = split(strFileValues, ";")
|
|
|
|
|
There's a third... Which I just dealt with today in an old Excel VBA macro (Essentially VB6)...
(Hastily-written, VB-style pseudo-code, as it's faster than removing the proprietary stuff)
For x = 1 to 13
Dim line As String
line = GetLine(x, username, code, thing, item, anotherstring, andanother, blah, blah, blah, blah, blah)
(Do something with line...)
Next x
...
Public Function GetLine(x As Long, username As String, etc etc etc etc etc)
Select Case x
Case 0
GetLine = "SomeString="&username
Case 1
GetLine = "Something Else=" & code
Case 2
GetLine = "Another Thing=" & thing
...
Case 13
GetLine = "Some literal string"
End Select
End Function
Yes, it passes all those parameters to the function 13 times. Yes, it only uses one of those parameters each time. Yes, every member of that select statement is a simple concatenation of a literal with a string parameter (If even that). Yes, I replaced the whole system with a C# utility as fast as I could.
|
|
|
|
|
VB6 is dead – I hope.
Too may surprise VB.Net is pretty good.
INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra
|
|
|
|
|
I'm sure you have all come across something like this. At least I see it way too often. So here it is in C# style:
<br />
bool someBoolValue;<br />
<br />
if (someBoolValue == true)<br />
{<br />
}<br />
else<br />
{<br />
}<br />
modified 19-Nov-18 21:01pm.
|
|
|
|
|
I occasionally find that slipping into my code. Looking back over it a few minutes later reveals my brain fart.
-------------------------------
Carrier Bags - 21st Century Tumbleweed.
|
|
|
|
|
That, and the classic ternary
(sMethod == "read"? true : false)
What I never understood is why these people stop so soon. The concept is readily extended...
if (doIt) => if (doIt == true) => if ((doIt == true) == true) ...
and combined
bool flag = ((doIt == true) == true? true : false);
if (flag != false) doIt = true;
We could go on all night...
|
|
|
|
|
Personally I don't find that to be horrible. It's completely correct and only slightly more redundant. It really makes no difference in practice.
And no I'm not trying to defend my usage of it I don't lie
|
|
|
|
|
I agree it's not horrible and that it can add to readability but it is still one comparison too many.
It might get more obvious in different situation.
Like (some stupid example):
<br />
<br />
bool IsNumberEven(int number)<br />
{<br />
return ((number % 2 == 0) ? true : false);<br />
}<br />
<br />
int Main()<br />
{<br />
int someNumber = 2^100;<br />
<br />
for (int i = 0; i < 100; i++)<br />
{<br />
while (IsNumberEven(someNumber) == true)<br />
{<br />
someNumber /= 2;<br />
}<br />
}<br />
}<br />
I think it is similar as using String object when one should use StringBuilder. It's OK for small number of iterations or single comparison, but can be time-consuming for large number of iterations.
modified 19-Nov-18 21:01pm.
|
|
|
|
|
I've always assumed that if(aBool) is simply a short-hand for if(aBool == true), and they should be completely equivalent to the compiler. If they are not for some reason, it would be a trivially easy optimization for the compiler to make.
|
|
|
|
|
I don't know if that is true. Maybe someone with deeper knowledge than me could answer that. If it is true though, I should 'recompile' my coding practices
modified 19-Nov-18 21:01pm.
|
|
|
|
|
I ran some tests for speed for both cases and the results show that your assumption is correct. It seems compiler is smarter than I thought So this case is closed and I learned something new to me and that is always a good thing
modified 19-Nov-18 21:01pm.
|
|
|
|
|
You ran a test for speed???
I put it in an app:
bool IsNumberEven(int number)
{
return ((number % 2 == 0) ? true : false);
}
private void Form1_Load(object sender, EventArgs e)
{
if (IsNumberEven(2))
{
}
if (IsNumberEven(2) == true) { }
}
checked with ildasm, and they both turn into exactly the same IL code.
.maxstack 2
.locals init ([0] bool CS$4$0000)
IL_0000: nop
IL_0001: ldarg.0
IL_0002: ldc.i4.2
IL_0003: call instance bool csharpWindowsApp.Form1::IsNumberEven(int32)
IL_0008: ldc.i4.0
IL_0009: ceq
IL_000b: stloc.0
IL_000c: ldloc.0
IL_000d: brtrue.s IL_0011
IL_000f: nop
IL_0010: nop
IL_0011: ldarg.0
IL_0012: ldc.i4.2
IL_0013: call instance bool csharpWindowsApp.Form1::IsNumberEven(int32)
IL_0018: ldc.i4.0
IL_0019: ceq
IL_001b: stloc.0
IL_001c: ldloc.0
IL_001d: brtrue.s IL_0021
IL_001f: nop
IL_0020: nop
IL_0021: ret
|
|
|
|
|
Testing if the number is even with modulus is the real horror .. :P
|
|
|
|
|
Actually the real horror would be:
<br />
bool IsEven(int someNumber)<br />
{<br />
if (someNumber == 2 || someNumber == 4 || someNumber == 6 ||
{<br />
return true;<br />
}<br />
else<br />
{<br />
return false;<br />
}<br />
}<br />
modified 19-Nov-18 21:01pm.
|
|
|
|
|
A rumour has it that it always can be worse
|
|
|
|
|
This might be even better. After all, it's the way I do it in my head.
bool IsNumberEven(int number) {
string sNumber = number.ToString();
string sLastDigit = sNumber[sNumber.Length - 1];
if(sLastDigit == '0' || sLastDigit == '2' || sLastDigit == '4' || sLastDigit == '6' || sLastDigit == '8' ) {
return true;
}
else {
return false;
}
}
|
|
|
|
|
It's ok e.g. in C#, but in C/C++ this habit can lead to nasty bugs.
If you work with multiple libraries, you probably have to deal with bool, BOOL and VARIANT_BOOL . While bool is fine (unless abused in the assignment), BOOL is usually used as "any nonnegative value is true", and the respective TRUE constant is defined sometimes as 1, sometimes a (!FALSE) which would expand to -1. VARIANT_BOOL explicitely defines -1 as VARIANT_TRUE.
I know that some people prefer
if (someridiculouslylongexpression==false)
over
if (!someridiculouslylongexpression)
since the not can get lost quickly for the quick reader. However, I discourage this habit, as it may fail with the "fak" boolean types.
|
|
|
|
|
dighn wrote: slightly more redundant
It is a way bit explicit right? Do you mean to say it affects any of the coding guidelines? C# actually advocates and prefers explicit typecasting too. And similarly an explicit mention should be a welcome though a bit strange. What do you way?
|
|
|
|
|
Actually I do not know why that was even posted, there was absolutely nothing wrong with that code.
Now if there was something in the “do something…” sections, that showed a problem – then it might have made since as a problem – otherwise it is great and legitimate code.
Tinko101 – can just ignore my comment, because I have only been writing code for about 19 years or so, but I agree with you.
INTP
"Program testing can be used to show the presence of bugs, but never to show their absence."Edsger Dijkstra
|
|
|
|
|
The only thing I can see wrong is that the boolean value isn't initialized. Now that is ban practice as in C/C++ values only get a default value in Debug mode. In Release mode it could be anything
|
|
|
|
|
I assumed the posted code was just missing an elipsis. Presumably there was something to set the Boolean value.
|
|
|
|