"If Not 'someCondition' Else" Logic

Question

I have a few co-workers who implement if-else logic in a way I've not seen before, and it kind of baffles me.

They write...

If <someCondition> Then
Else
    Console.WriteLine("Hello, World")

...instead of

If Not <someCondition> Then
    Console.WriteLine("Hello, World")

This got me thinking

Is there any difference between the two approaches? Is one approach more efficient than the other?

I doubt there's any difference efficiency-wise but this seems like a code smell to me. — rory.ap, Oct 08 '15 at 13:58
Some people personally prefer that conditions reflect positive concepts rather than negative ones, it's often easier to read them at a glance. I guess some people prefer it strongly enough to not also be bothered by empty `If` blocks. (I agree with you, I'd rather use the negative condition than have weirdly structured blocks.) If there is a performance difference, it's likely so negligible that there are *much bigger fish to fry* in terms of performance tweaking. — David, Oct 08 '15 at 14:05
I do this quite a lot, particularly if the condition is complicated; positive conditions are easier to understand. But I'd be inclined to add a comment to the empty block `' do nothing` to make it clear I hadn't just forgotten to put something in. — Brian Hooper, Oct 08 '15 at 14:41
Readable workaround can be `If = false Then ...`. In some cases `Not` in front of condition can break a readability flow. That's why in `vb.net` exists `IsNot` and `TypeOf ... IsNot` — Fabio, Oct 08 '15 at 15:17
@Fabio: I would think `If = false Then ...` would include a _very_ small performance hit because ` = false` requires evaluating both sides of the `=` while `Not ` only requires evaluating one variable. But I would think the change would be so minuscule that its not really worth mentioning outside of nitty gritty performance discussions. — Mike_OBrien, Oct 08 '15 at 15:21
I personally hate these extra empty blocks but our code base has a lot of these. Although, I've been removing it vigorously. I think, this is not only that some people like to have positive condition, it is also a `lavaflow` anti-pattern - people remove some code while leaving stuff behind. — T.S., Oct 08 '15 at 20:23
@BrianHooper thumbs up for `' do nothing`. This is exactly what need to happen in empty blocks like this. I have this often in `catch` blocks. Like those, where you really can't do anything about the problem. Without a comment, it looks like foolish coding. At leas comment shows the intent. thank you — T.S., Oct 08 '15 at 20:32
@Mike_OBrien, `If Not Then ...` and `If Then ...` will produce same IL code without performance difference. Do same steps(test) as @Bjørn-Roger Kringsjå — Fabio, Oct 10 '15 at 04:25
@Fabio I did not test it at all so my opinion was strictly assumption based. Having the empirical evidence makes it a much simpler situation. +1's all around for actually doing the leg work. — Mike_OBrien, Oct 11 '15 at 05:02

score 3 · Accepted Answer · answered Oct 09 '15 at 05:04

Question: "Is there any difference between the two approaches?"

Answer: No, there is no difference.
Question: "Is one approach more efficient than the other?"

Answer: No, they are both equally efficient.

How do I know this?

Well, in situations like this it's easy to determine the difference by creating a test project and look at the compiled IL (release config).

I created a console application using Visual Studio Community 2015 with .NET 4.5.2. And as a decompiler I used .NET reflector (a free alternative is ILSpy).

VB.NET

Public Sub Test1(condition As Boolean)
    If (condition) Then
    Else
        Console.WriteLine("condition was false.")
    End If
End Sub

Public Sub Test2(condition As Boolean)
    If (Not condition) Then
        Console.WriteLine("condition was false.")
    End If
End Sub

Console output:

condition was false.
condition was false.

IL

As stated before, and confirmed below, both methods are identical.

.method public static void Test1(bool condition) cil managed
{
    .maxstack 8
    L_0000: ldarg.0 
    L_0001: brtrue.s L_000d
    L_0003: ldstr "condition was false."
    L_0008: call void [mscorlib]System.Console::WriteLine(string)
    L_000d: ret 
}

.method public static void Test2(bool condition) cil managed
{
    .maxstack 8
    L_0000: ldarg.0 
    L_0001: brtrue.s L_000d
    L_0003: ldstr "condition was false."
    L_0008: call void [mscorlib]System.Console::WriteLine(string)
    L_000d: ret 
}

This is excellent technical answer. But the answer why they do it - is a psychological component — T.S., Oct 09 '15 at 16:29
@T.S. Thank you. The question must have been edited within the grace-period because when I read it there was no *"why"*, This also explains why someone would VTC as primarily opinion-based (followed by a few sheeps VTC even after the question were edited into shape) — Bjørn-Roger Kringsjå, Oct 09 '15 at 16:41

score 0 · Answer 2 · answered Oct 08 '15 at 20:28

Let me risk to answer this:

There is nothing wrong with either way. It is highly opinion-based. And I will not be offended if your question will be closed for that very reason.

I believe, there are 3 main reasons for this to appear

People like positive condition to be listed first, empty or not
People wrote code because they thought that there will be positive condition
People removed code from positive condition later while left if-else construct intact

score 0 · Answer 3 · answered Oct 08 '15 at 23:48

0

Both approaches are correct. There is no noticeable improvement of one over the other.

The reason why sometimes people code like that is for the sake of simplicity. Sometimes logic can get confusing if you use too many negations.

For example: "We don't need no education" actually means "We do need education"

Also sometimes coders get structures like that preparing for future amends of the logic.

answered Oct 08 '15 at 23:48

El Gucs

897
9
18

"We don't need no education" is double-negative, which I wrote as forbidden into our coding standards – T.S. Oct 09 '15 at 01:53

score 0 · Answer 4 · answered Oct 09 '15 at 02:41

First, there is no performance implication, so don't let that bother you. Secondly, if there was a performance implication, it would be too small to worry about in all but the most extreme circumstances. So forget about performance, this is not about performance.

So, the question as to why this might be done: in order to clearly identify intent. It shows that both branches have been considered. I have personally done this in the rare circumstances where I want to make it crystal clear that the right thing to do is: nothing. If the do nothing branch isn't the most important thing to know at that point, don't do this.

"If Not 'someCondition' Else" Logic

4 Answers4