Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal: Conditional control statements #724

Closed
jamesqo opened this issue Jul 5, 2017 · 4 comments
Closed

Proposal: Conditional control statements #724

jamesqo opened this issue Jul 5, 2017 · 4 comments

Comments

@jamesqo
Copy link
Contributor

jamesqo commented Jul 5, 2017

Background

It's pretty common to see control statements such as break, continue, return, etc. inside a branch of a loop. For example, the following is a common idiom that ensures a condition is true before continuing the loop.

foreach (var item in list)
{
    if (!condition(item))
    {
        break;
    }

    // Do work knowing that the condition is true.
}

Proposal

One thing that struck me is the logic to break out of the loop seems more verbose than necessary. Since this is such a common pattern, wouldn't it be nice if we could just write

// The `break` is executed conditionally, if `!condition(item)` is true.
// Parentheses are required.
break if (!condition(item));

? Right? No new keywords introduced, and we eliminate 3 really sparse lines of code ({, break;, }) that taking up precious screen real estate. I believe Python has similar syntax to do this, e.g. you can write return foo if bar else baz, raise some_error if condition, etc. I think this would be a logical feature for C# to adopt, too.

More usage examples

        // Before:
        public static bool IsNullOrWhiteSpace(String value)
        {
            if (value == null) return true;

            for (int i = 0; i < value.Length; i++)
            {
                if (!Char.IsWhiteSpace(value[i]))
                {
                    return false;
                }
            }

            return true;
        }

        // After:
        public static bool IsNullOrWhiteSpace(String value)
        {
            return true if (value == null);

            for (int i = 0; i < value.Length; i++)
            {
                return false if (!Char.IsWhiteSpace(value[i]));
            }

            return true;
        }
// Before:
if (arg == null)
{
    throw new ArgumentNullException(nameof(arg));
}

// After:
throw new ArgumentNullException(nameof(arg)) if (arg == null);
        // Before:
        public IEnumerable<ProjectReference> GetAddedProjectReferences()
        {
            var oldRefs = new HashSet<ProjectReference>(_oldProject.ProjectReferences);
            foreach (var reference in _newProject.ProjectReferences)
            {
                if (!oldRefs.Contains(reference))
                {
                    yield return reference;
                }
            }
        }

        // After:
        public IEnumerable<ProjectReference> GetAddedProjectReferences()
        {
            var oldRefs = new HashSet<ProjectReference>(_oldProject.ProjectReferences);
            foreach (var reference in _newProject.ProjectReferences)
            {
                yield return reference if (!oldRefs.Contains(reference));
            }
        }

List of control statements that will be affected

  • break
  • continue
  • goto
  • return
  • throw
  • yield break
  • yield return
  • Feel free to comment if you think I'm missing any.

Extra Notes

  • It's true that some people may not mind writing if (!condition(item)) break; in the first place, which is just as short as break if (!condition(item));. However, for those who prefer to use braces or even leave the break statement on the next line, there is a significant difference in concision.
@HaloFour
Copy link
Contributor

HaloFour commented Jul 5, 2017

Seems completely unnecessary given you can already use all of those control statements as the embedded statement of a condition:

if (condition) break;
if (condition) continue;
if (condition) goto foo;
if (condition) return;
if (condition) throw new Exception();
if (condition) yield break;
if (condition) yield return value;

The new proposal isn't at all more concise. All it does is satisfy the excessively OCD who feel that their code is somehow "dirty" if they don't follow a conditional with a block. It's another way to do something for the sake of another way to do something.

@Vlad-Stryapko
Copy link

Reminds me of Ruby. However, I don't really think it's worth it. First of all, it doesn't add anything extremely useful. Furthermore, we'll have two ways of expressing the same idea without any semantic difference so it needs to be covered by project code guidelines or otherwise the code will be written differently by each person.

@jamesqo
Copy link
Contributor Author

jamesqo commented Jul 5, 2017

@HaloFour I mentioned your point in the last section. I kinda disagree with you because I think consistency (by adding blocks after conditionals) is important, but I have a feeling other people will side with you (13 minutes in, 2 thumbs-down already). I'll close this issue.

@jamesqo jamesqo closed this as completed Jul 5, 2017
@svick
Copy link
Contributor

svick commented Jul 5, 2017

In Ruby, lines equivalent to the following kept confusing me:

throw new ArgumentOutOfRangeException("Some exception message, that is very long.", nameof(parameter)) if condition;

When scanning code, I start reading from the begging of each line. If I see that the line starts with throw, I will expect that it unconditionally throws and might overlook the if at the end of the line.

It's even worse here on GitHub, I have to scroll to see the if.

That's why I'm against this proposal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants