0

I often find that "!" characters (meaning "not" in C-based languages) are easy to overlook. So I sometimes create methods that return the inverse result.

For example, instead of doing this:

if (!dataContext.Permissions.HasAccessToFeature("FeatureX")) { ... }

...I create a second method that is the inverse of the other, making the code read as follows:

if (dataContext.Permissions.DeniedAccessToFeature("FeatureX")) { ... }

I feel this makes the code more readable, but I'm wondering if others agree?

  • That's why some C++ programmers prefer the alternative keyword not over the ! operator but I don't think that C# has an equivalent feature. – 5gon12eder Jun 21 '16 at 22:43
10

I think the second variant could lead to more confusion. true should imply positive results, and false a negative result. The question in natural language is usually "Are you allowed to access the feature?", not "Are you not allowed to access the feature?".

The response then is also consistent - "Yes you are allowed to access the feature" vs "Yes, you are not allowed to access the feature".

What happens in your second case when you want to check for the opposite result, where a user does have access to the feature?

if (!dataContext.Permissions.DeniedAccessToFeature("FeatureX"))

If the ! operator really confuses you that much, why don't you just do

if (dataContext.Permissions.HasAccessToFeature("FeatureX") == false)

Regarding one part of your question:

...I create a second method that is the inverse of the other

On a meta level, do you really think it is wise to have two different methods which perform the same task? What happens if the logic needs to change, but only one method is updated because your inverse method is only used in a few places?

  • 3
    ++ but the inverse method could be defined in terms of the positive case. return !Permissions.HasAccess(user); I can see where it could make for a nice read. – RubberDuck Jun 21 '16 at 13:46
  • RubberDuck, that's what I envisioned, correct. – willem Jun 21 '16 at 15:49
  • 6
    Adding "== false" is less clear, because most programmers will say "why? why? why?" – kevin cline Jun 21 '16 at 18:33
  • @kevincline, I agree completely – user1666620 Jun 21 '16 at 22:20
9

Let's consider both the general and the specific problem.

In general the advice is usually to avoid negatives in names. The reason being: you note that it can be confusing to negate a thing; well, sometimes you have to negate a thing, and it is very confusing to negate a negative:

if (!product.NotTaxable) ...

Yuck.

In the specific case you mention there is specific advice. Your proposal has usability problems that have security implications, in at least two ways.

First, in most security systems you want to default to the secure mode, not the insecure mode. That is, you say "access is disallowed by default; having a permission granted is special, and allows access". You do not say "access is allowed by default; having a permission denied is special, and disallows access". So you want to design your code in the same way, to emphasize this.

Second, many security systems do not have a "binary" relationship between "granted" and "denied". That is, just because a particular permission is granted, does not imply that it is also not denied!

Imagine for example a permission system that grants access to the resource to the accounting group, but denies access to employees on vacation. Bob is an accountant on vacation; he is both granted access AND disallowed access, and now you have to decide which one wins. Again, probably you want to default to the secure mode and disallow access.

Or imagine a permission system where there are three kinds of rules: some grant access, period, some deny access, period, and some say "I don't apply to this situation". You make a priority list of rules and you run down it until you get the first grant, in which case you're done, or the first deny, in which case you're done. Here again we have the property that "not granted" and "denied" are very different.

The important thing is not whether you are avoiding !s, but rather that your APIs are very clearly and correctly expressing the logic of the business domain. That's the real readability concern.

3

According to Uncle Bob

Negatives are just a bit harder to understand than positives. So, when possible, conditionals should be expressed as positives.
For example:
if (buffer.shouldCompact()) is preferable to if (!buffer.shouldNotCompact())

From Clean Code Chapter 17, Item G29 "Avoid Negative Conditionals"

So your first example is the preferable one.

However, he himself does not give an explanation, but I completely agree with him here.

  • 2
    "Negatives are just a bit harder to understand than positives" is the explanation... – svidgen Jun 21 '16 at 16:11
  • @svidgen you're right, I actually meant "source", not "explanation" – Lovis Jun 22 '16 at 6:46
1

I would not make a function for the sole purpose of renaming a ! operator because that's a complete disaster when it comes to interface minimalism and code maintenance with two different conventions, but if you have the choice from the start, make functions the way that's the most obvious to use. To me, that would be using the second form (and only this one), because most likely you will repeatedly use if (PermDenied(feature)) throw ... and almost never need to negate it.

Again, this is just following my own zen of fighting against length and useless operators ; but verbose naming with simple logic can only be better than verbose logic with simple naming.

  • Even if this means you now have two methods with the same logic but which return different boolean values? Ref the following from the question: "...I create a second method that is the inverse of the other" – user1666620 Jun 21 '16 at 13:10
  • Made an edit to be more clear about two functions problem – Arthur Havlicek Jun 21 '16 at 13:15
1

If you check whether access is denied, then there isn't much difference between "! hasAccess" and "deniedAccess".

However, if you check that you have access, then there is a huge difference in readability between "hasAccess" and "! deniedAccess". You need to take both cases into account when you decide how to name a method or property.

1

As others have said, the advise is to avoid negatives while naming a method. That should be easy to follow in any type of language.

But I would like to go into the "!" question. You've said it is easy to overlook and I agree and disagree at the same time. I think for people new to C# that are not used to this kind of syntax yes, it's easy to overlook. For people used to C# or this kind of syntax, the first thing they will notice is exactly the presence or absence of the "!".

Have been developing C# for many years now I didn't even thought much about it until I've read your question and noticed that I looked immediately for the beginning of the condition to see whether or not the "!" was there.

My advice is to use "!", because in my opinion, most C# developers are used to it. But in the end what really matters is you and your team agreeing if you should use it or not and be consistent about it in all your source code.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.