Redundancy Peaking

Lost User

Why? Why? Why check an already boolean result? Noticed this redundancy in many parts of an ill written app.

        If myControl.Visible = True Then
            'Some Code
        Else
            'Some other code
        End If

I don't know why, it's just plain turn off to see this redundancy!

- Just that something can be done, doesn't mean it should be done. Respect developers and their efforts! Jk

Allan Thomas

Unfortunately this is the biggest issue I have about vb. It's too easy to be deep in thought and literally code what you are thinking and make the code more annoying to read. i.e. If control x visible property is true then do this and this and this. I usually pick it up after I have written it and clean it up afterwards but sometimes I've come across code a few months later and go whoops. Hopefully the compiler is smart enough to fix the extra redundancy I've added so it doesn't effect performance.

Sander Rossel

Though not my preferred style of writing code I don't see how this is such a horror. It is readable, it is clear, no one could ever interpret it wrong (not even newbies to programming!). When I just started out I remember seeing something like If someVar Then and I was looking what someVar could be and if it should be True or False (it should be something!? :confused: )... Sure, it is some redundancy, but if my computer is acting up and I can hear it having a hard time I am quite sure it is never because some programmer checked for True ;) If checking Booleans for True like that is the biggest 'horror' in the code I sometimes encounter I would be out of a job! ;) What is more of a horror to me is first checking if a Boolean is Not True, like this:

If Not myControl.Visible Then
' Some Code
Else
' Some other code
End If

That would of course only be justified if there was no Else ;) But even that is not so much of a coding horror, just annoying. By the way, if this app is really ill written I'd like to see some other cool stuff! :D

It's an OO world.

User 2558377

I don't see why this is specifically a problem with VB - exactly equivalent code can be and often is written in C# and other languages.

Anyway, to be really sure the control is visible you should obviously write

If (myControl.Visible = True) = True Then...

so it doesn't effect performance
You mean "affect" (I hope)

Nikola Radosavljevic

I actually sometimes check is some condition is not fulfilled even when there is else branch. This I do in cases when one case of if/else block is expected behavior and in other one i do simple logging/recovery or similar thing. Specifically, I do this when one block is short (less than 5 lines), and put that block in front. In that case, code is more clean to my eye because else block is very near to if block. This is very helpful to me when i have several nested if/else structures. Would that make sense to anyone but me?

MikeD 2

makes sense to me to have the preferred condition as the first block of code if not WingsFallenOff then FlyNormally else Panic End if

mdblack98

It's not redundant at all...it's explicit. And there's also a difference between "= True" and not leaving it empty. Any non-zero value is "True" but "True" is not any non-zero value. There's no extra code generated as this is what is actually done when you leave off the "= True" part. Given there's no difference in the generated code I fail to see your problem. Personally, I prefer explicit coding as you're much less likely to make a mistake. How's this for redundant? if (a==1) { cout << "a=1" << endl; } Braces are completely redundant...but you'll never bite yourself by forgetting to put them in when you have to expand the clause in the future.

streamcap

In C++, it's explicit. a bool variable can be zero or non-zero, making the check at least desirable. In VB, or C#, or Java, it's unnecessary most of the time, since a bool can only have two values - and defaults to false instead of null.

Nagy Vilmos

How about:

try{
if (wingsFallenOff) {
throw new OutOfLiftError("Ahhhhhhhhhhh!!!!!");
}
flyNormally();
} catch (OutOfLiftError ex) {
panic(ex);
}

To be honest, rather then checking the boolean value, a method call, say checkAirworthy(), could be used that, if the plane is in the air, will throw an exception rather than returning false. Thta means that you can later upgrade it to check for engines, air-con and lemon-scented hand sanitisers. The same method would then be used before pushing back.

Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

Nikola Radosavljevic

It may seem structurally sound, but it's a mistake to use exception handling for control flow. First, exceptions are exactly that. Cases which you do not expect should happen in normal workflow, exceptional situations which are caused by a failure in the system. They should not be used to control business logic. However, I am also tempted to use them in this way when I have more than 2 layers of abstraction between business logic decision, and handling of that decision. Second, it's big hit to performance. If this is not client side code, or a single occurrence during request processing, this can be real PITA when you come to a stage you need to optimize performance.

BrainiacV

Yes, but how will we know it's True unless we check? :laugh: It could be a half truth.

Psychosis at 10 Film at 11 Those who do not remember the past, are doomed to repeat it. Those who do not remember the past, cannot build upon it.

Nagy Vilmos

Nikola Radosavljevic wrote:

exceptions are [...] Cases which you do not expect

So, you expect the wings to fall of the plane? Unless they're upgrading the TU-154[^] to fly-by-wire, I don't think it is /normal/ behaviour. :-D

Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

Nikola Radosavljevic

:D True. I didn't expect myself properly, but still. If user enters invalid PIN code, would you use exception to handle this situation? I'd argue you shouldn't.

Nagy Vilmos

The PIN case is different. When a PIN is entered, it has to be authorised or declined. There is no exception in this. If the software has used an invalid key for the encryption then THAT could be an exception, in general in is treated as decline.

Panic, Chaos, Destruction. My work here is done. Drink. Get drunk. Fall over - P O'H OK, I will win to day or my name isn't Ethel Crudacre! - DD Ethel Crudacre I cannot live by bread alone. Bacon and ketchup are needed as well. - Trollslayer Have a bit more patience with newbies. Of course some of them act dumb - they're often *students*, for heaven's sake - Terry Pratchett

Nikola Radosavljevic

PIN is more similar to usual business cases than wings falling off a plane :) Anyhow, maybe I could make up a rule that would say: Do not use exception handling if result of failed operation is displayed to end user, and end user is expected to understand it (there can be exceptions to this :) )

Lost User

Well, I use to resolve this kind of problem with a Ctrl+H, replacing '= True Then' by 'Then', simply. I hate a dirty code....

Ygor Lazaro

V 0

Don't know about VB, but in C++ a boolean can sometimes have other values then true / false that would go through if you did something like this:

if(boolvalue){
// code here
}

but would do it correctly when doing this:

if(boolvalue == true){
// code here
}

V.

wbaxter37

I confess that I tend to do this, but I do have a reason. I program in multiple languages. Not explicitly calling out an equality in a dynamic and loosely typed language like Ruby can give you unexpected results. A variable will evaluate to true if it is true or has an assigned value (even if it's an empty string). It will evaluate to false if it is false or nil. In the interest of clarity I tend to make logical tests explicit and let the compiler take care of things. Turbo C 1.5 optimized this way back when, so I'm sure VS2010 can handle it. There's no harm in being explicit in one's source code. It's like using "extra" parentheses. It doesn't matter to the tools, but it sure does make reading the code easier for us humans. Not to say i don't appreciate obfuscated C, it's just I do't want to have to modify it and make it work.

mdblack98

And I ran this test once with "if mybool" and once with "if mybool = True". No difference in speed. 1st one showed 33 seconds, 2nd one showed 32 seconds.

Module Module1

Sub Main()
    Dim mytime As Date
    Dim mybool As Boolean
    Dim mydiff As TimeSpan
    Dim j As Double
    mytime = TimeOfDay()
    While mytime = TimeOfDay()
    End While
    mybool = True
    j = 0
    For i = 1 To 100000
        For j = 1 To 100000
            If mybool = True Then
                j = j + 1
            End If
        Next
    Next
    mydiff = TimeOfDay() - mytime
    Console.WriteLine(mydiff)
    Console.ReadKey()
End Sub

End Module

Ralph Little

Agreed. Anyway, writing

if(boolvalue == true){
// code here
}

is like saying "if boolvalue is true is true" :D