Proposal: Boolean Literal Aliasing

[gafter]

@jcdickinson commented on Wed Feb 27 2019

Rationale

Disclaimer: I'm not sure if this is a good idea. All ideas are worth sharing.

There is a thought school of developers who believe that booleans are evil. Once piece of code I previously worked with had some 10 boolean arguments, leading to code like this:

wizard.ShowDialog(host, ..., true, false, false, false, false, true, false, true, true);

A more relatable example for most is easy to find: StreamWriter.

new StreamWriter("C:\\tmp\\tmp.txt", true, Encoding.UTF8);

What does that true mean? Finding out might involve re-typing the opening parenthesis, goto definition or opening the documentation, depending on what your editor is capable of doing. There are also security implications. Don't Google-er-Bing this, off the top of your head, what does this mean?

SslStream ssl;
ssl.AuthenticateAsServer(certificate, true, false);

Opponents of booleans recommend either separate methods (StreamWriter.CreateAppend vs. StreamWriter.CreateNew) or enums (new StreamWriter(AppendOptions.Append or new StreamWriter(AppendOptions.New)).

An existing way to deal with this problem would be to:

new StreamWriter("C:\\tmp\\tmp.txt", append: true, encoding: Encoding.UTF8);

The problem with that approach is that "all arguments following a named argument must be named." It adds friction to clarifying code in this way.

We can't simply remove booleans from the framework. It would be nice to go over netcore and allow boolean call sites to document themselves.

Solution

The following constructor prototype (the syntax could use work):

StreamWriter(string path, bool<Append, Clear> append, Encoding encoding)

Is compiled to the following actual constructor prototype:

StreamWriter(
  string path,
  [System.Runtime.CompilerServices.BooleanAlias("Append", "Clear")]
  bool append,
  Encoding encoding);

It can then be used in one of the following ways:

new StreamWriter("", bool.Append, null); // Equivalent/compiled to:
new StreamWriter("", true, null); // Is still valid in the face of boolean aliases

new StreamWriter("", bool.Clear, null); // Equivalent/compiled to:
new StreamWriter("", false, null); // Is still valid in the face of boolean aliases

Treating bool like an enum has two advantages:

• Existing code continues to work when, for example, a local variable or field shares the name with an alias.
• In scenarios where a third option is added, the alias can be promoted to an enum during refactoring with, at worst, find and replace. A Rewriter might be able to automate the whole thing for Omnisharp/VS.

Other Concerns

Would the following be desirable?

public bool<Found, NotFound> TryGetValue(TKey key, out TValue value);
if (dictionary.TryGetValue(key, out string value) == bool.Found) { }

That smells of the if(true == true) redundancy, but it does somehow add value in this case. The existing compiler machinery for named tuple elements could be re-used.

---

@CyrusNajmabadi commented on Wed Feb 27 2019

A more relatable example for most is easy to find: StreamWriter.

new StreamWriter("C:\\tmp\\tmp.txt", true, Encoding.UTF8);

What does that true mean?

This is pretty simply solved by just writing:

new StreamWriter("C:\\tmp\\tmp.txt", append: true, Encoding.UTF8);

The IDE even offers to do this for you if you use the quick-actions feature:

---

@CyrusNajmabadi commented on Wed Feb 27 2019

off the top of your head, what does this mean?

SslStream ssl;
ssl.AuthenticateAsServer(certificate, true, false);

it means this:

            ssl.AuthenticateAsServer(
                certificate, clientCertificateRequired: true, checkCertificateRevocation: false);

That seems pretty easy to understand. It's self documenting. And it's safe to someone even switching around the order of things.

---

@CyrusNajmabadi commented on Wed Feb 27 2019

The problem with that approach is that "all arguments following a named argument must be named." It adds friction to clarifying code in this way.

This is not true. See dotnet/roslyn#33753 (comment) as an example :)

---

@CyrusNajmabadi commented on Wed Feb 27 2019

if (dictionary.TryGetValue(key, out string value) == bool.Found)

if (dictionary.TryGetValue(key, out string value)) { is already idiomatic. It really doesn't need any changes.

---

@gafter commented on Wed Feb 27 2019

Moving to csharplang as this is a language feature request.

[Joe4evr]

There is a thought school of developers who believe that booleans are evil.

Well, these people are stupid. 🙃

Once piece of code I previously worked with had some 10 boolean arguments, leading to code like this:

It sounds more like this codebase simply wasn't making proper use of existing and much more viable alternatives. In this case, they should've realized creating a flags enum is much more efficient, and the official design guidelines state as much. There's no need to introduce some new arcane syntax for something that is easily doable with the simplest kind of type: an enum.

Booleans are great to represent a 2-state choice, but when more choices are needed in your system, you should be flexible enough to redesign it, lest you keep on tightening bolts with a hammer.

[Austin-bryan]

The issue here is you're trying to use bools like they're enums. First off, if you used proper naming convention, this would already become more clear. if (append) or DoSomething(append: true) both don't make nearly as much sense as if (shouldAppend) or DoSomething(shouldAppend: true).

Regardless, even if your enum has only two values (Append, Clear) that should still be an enum because these values aren't true and false, therefore a container which I can use to move them about is much nicer and easier.

Also, the code isn't that hard to write, nor space-taking:

public enum Foo { Append; Clear; }

Also, the overall syntax of bool.Foo is worrisome and highly confusing. It indicates we are, again, misusing bool. The fact that we can use either bool.Append or true would make the code even more confusing to read, as someone could just as easily use true and the future reader would, again, not know what that means, thereby not even solving the original issue. While this is the best way to not break old code, it adds language complexity but doesn't solve the problem.

Basically if you need more context and meaning behind bools, that's what enums are for.

[DavidArno]

There is a thought school of developers who believe that booleans are evil.

Whilst that school of thought may well exist, I think you have perhaps confused it with the view (fact even) that boolean parameters are generally a bad idea. "Evil" is perhaps taking things a little far though.

[svick]

@jcdickinson

The problem with that approach is that "all arguments following a named argument must be named." It adds friction to clarifying code in this way.

As has been pointed out by @CyrusNajmabadi, this is no longer true. You might not be aware of that, since that change only happened in C# 7.2.

[kharai-tor]

I'm not a huge fan of this idea and I agree with the opinion that using the named argument makes it clear enough.

However, if I had to come up with a solution to this, I would propose the following:

enum StreamWriterOptions : bool
{
    Append = true,
    Clear = false
}

Both of these two invocations would compile:

new StreamWriter("C:\\tmp\\tmp.txt", StreamWriterOptions.Append, Encoding.UTF8);
new StreamWriter("C:\\tmp\\tmp.txt", true, Encoding.UTF8);

But the IDE would issue a warning on the latter and suggest a code fix.

Anyway, I still don't think it's worth the effort.

[TheUnlocked]

I think that this would mainly just encourage bad code. If you have ten boolean arguments, you should probably be using a single int argument with a bit flag enum instead.

Now, a feature to super easily unpack an int's bits into a bunch of booleans could be an interesting proposal, but that would belong in its own issue.

[Joe4evr]

Now, a feature to super easily unpack an int's bits into a bunch of booleans could be an interesting proposal, but that would belong in its own issue.

When #297 becomes a thing, you could just add a Deconstruct() method that does exactly that.

[HaloFour]

@Joe4evr

Why wait? You can write extension Deconstruct methods now.

[Joe4evr]

Oh yeah! Thanks for the reminder.

[theunrepentantgeek]

There is a thought school of developers who believe that booleans are evil.

I'm certainly of the school of thought that boolean parameters are evil. I've even blogged about it more than once.

So I'm really a fan of eliminating random true and false values that require investigation to understand, but I'm not too keen on the proposed syntax shown above.

If you have control over the method declaration, it would be better to change the type to use an enum, as has already been suggested.

For example, the original example could be rewritten by declaring an enum:

public enum StreamWriterOption { Append; Clear; }

And then the constructor would be:

StreamWriter(string path, StreamWriterOption append, Encoding encoding)

What would be nice would be a way to allow the name of the enumeration type to be omitted when calling the method ... so instead of needing to write ...

new StreamWriter("C:\...\foo.txt", StreamWriterOption.Append, Encoding.UTF8)

we could write

new StreamWriter("C:\...\foo.txt", Append, UTF8)

Perhaps this could be an attribute?

StreamWriter(
    string path, 
    [AssumeEnumType]StreamWriterOption append, 
    [AssumeEnumType]Encoding encoding)

This would be a language change that doesn't change any of the syntax ...

[yaakov-h]

@theunrepentantgeek #354

[jcdickinson]

@Joe4evr: Well, these people are stupid.

There are some really well respected folks who hold this opinion, even if they weren't, they are still humans:

README.md: This project has adopted the code of conduct defined by the Contributor Covenant to clarify expected behavior in our community.

@willard720: Regardless, even if your enum has only two values (Append, Clear) that should still be an enum because these values aren't true and false, therefore a container which I can use to move them about is much nicer and easier.

While this is true, that interface (Append/Clear) I provided comes directly from the BCL.

@svick @CyrusNajmabadi: C# 7.2

That probably renders this discussion pointless. @gafter Feel free to close this off.

[theunrepentantgeek]

Thanks @yaakov-h, I hadn't seen that one. Nice.

[dsaf]

Any modern IDE gives you this automatically:

[orthoxerox]

@dsaf not everyone works with code in an IDE. Code review is often done in a different tool. On the other hand, you can add StyleCop that auto-rejects naked boolean literals in such cases.

[MgSam]

I think the crux of this suggestion is akin to declaring inline enums, which itself is not an unreasonable request. It can be overly verbose to declare a whole separate enum (in a whole separate file) just to be able to accurately specify the arguments to a single method.

TypeScript has a concept of string-literal types, which can function a lot like an inline enum. It allows your method signature to declare the complete set of acceptable values for a given parameter.

[DavidArno]

@dsaf,
That so shows the truth of "you can't please all of the people all of the time". R# added that feature recently. It survived on my machine only for as long as the time taken for me to work out how to turn it off. Awful, in-your-face, noise in my view. 🤷‍♂️

[dsaf]

@DavidArno yeah, it's not perfect. Still is it that worse than VS code lens thing? Eventually I might just go back to only using https://github.com/MrJul/ReSharper.EnhancedTooltip .