CODEWITHSUNDEEP
c#.net
silla
silla

Reputation: 1327

How can I write this if with || , && shorter?

what if I would have something like this:

if (data == null || (data != null && (data.Count() != 3 || data.IsNotCorrect()))
{
  //error...
}

The data == null and then || (data != null) part is somehow ugly. Is that how to solve this problem in c#?

Edit: Sorry! Changed data.IsCorrect() to data.IsNotCorrect()

Upvotes: 1

Views: 217

Answers (5)

jgauffin
jgauffin

Reputation: 101150

Well. data.Count() != 3 || data.IsCorrect(). What you are saying is that the data isn't correct if the count is 3. So why the extra if condition instead of doing that check internally? 

if (data == null || data.IsCorrect())
{
  //error...
}

Update

Seems to be confusion about what I mean with moving the validating logic to inside the class. All classes should be responsible of determine what's a valid state or not in proper OOP. It's called encapsulation.

This whole idea of letting the caller instead of the class itself determine what's correct or not have emerged more and more since the introduction of OR/Ms, mappers etc etc. But the fact is that classes which doesn't validate their own information is not really properly designed OOP classes. They are just containers like DTOs.

The danger with that is that each and every block of calling code is responsible of making sure that the DTO contains correct and valid information. That means that there are n places in your code where a bug can be introduced instead of just 1.

So that's why I recommend that you move all validation logic to inside the IsCorrect or whatever you like to call it. But if you really want to code according to the basic OOP principles you should not let the class to get in an inconsistent state ever. And that's done as I describe in the blog post below.

http://blog.gauffin.org/2012/06/protect-your-data/

Upvotes: 8

CodesInChaos
CodesInChaos

Reputation: 108800

The right side of || gets only evaluated if the left side isn't true (|| is short-circuiting)

The conditional-OR operator (||) performs a logical-OR of its bool operands. If the first operand evaluates to true, the second operand isn't evaluated. If the first operand evaluates to false, the second operator determines whether the OR expression as a whole evaluates to true or false.

Quoted from || Operator (C# Reference) (MSDN)

Apply that to your expression:

data == null || (data != null && (data.Count() != 3 || data.IsCorrect())

This means when data != null is reached, data is guaranteed to be different from null, and thus data != null is guaranteed to be true, and you can leave it out.

Thus your expression is equivalent to:

data == null || data.Count() != 3 || data.IsCorrect()

Upvotes: 3

Christofer Eliasson
Christofer Eliasson

Reputation: 33865

Just get rid of the data != null part. Since you already gone passed the first part of the statement, you know by that time that data is not null, so no need to check it.

if (data == null || (data.Count() != 3 || data.IsCorrect()))

Upvotes: 1

Jon
Jon

Reputation: 437376

You can remove the second data != null check because of lazy evaluation, and then you no longer need parens because you are left with only || operators:

if (data == null || data.Count() != 3 || data.IsCorrect())

Of course the above is slightly suspect (I would have expected to see !data.IsCorrect()).

Upvotes: 5

Rawling
Rawling

Reputation: 50114

You can just leave off the (data != null && ... ) (i.e. leave the ... part).

If the argument before || evaluates to true (i.e. data is null), the argument after || is not evaluated - so you don't have to worry about null reference exceptions. This is called short-circuiting.

Upvotes: 7

Related Questions