Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Add for/while else #143

Open
pauldraper opened this issue Sep 29, 2020 · 6 comments
Open

Add for/while else #143

pauldraper opened this issue Sep 29, 2020 · 6 comments

Comments

@pauldraper
Copy link

pauldraper commented Sep 29, 2020

I tried to use for-else, only to discover it does not exist (syntax error at 'else': expected expression).

for root in roots:
    if src.path.startswith(root):
        modules.append(create_module(name = src.path[len(root):], file = src))
        break
else:
    fail("Module roots does not contain %s" % src.path, "srcs")

This is surprising to someone familiar with Python control structures.

@alandonovan
Copy link
Contributor

Yes, Starlark does not support for/else, and this may be surprising to a Python programmer. On the other hand, a programmer familiar with every language but Python might stop and wonder what for/else means. Between that, and the complexity of adding "more stuff", I don't find it a compelling feature to add to the language when there is an efficient and abundantly clear workaround available: a boolean variable.

@pauldraper
Copy link
Author

pauldraper commented Sep 30, 2020

On the other hand, a programmer familiar with every language but Python might stop and wonder what for/else means.

They have a lot of stuff to wonder about too, like significant whitespace(!!!).

With the abundantly clear workaround,

for root in roots:
    if src.path.startswith(root):
        modules.append(create_module(name = src.path[len(root):], file = src))
        break
else:
    fail("Module roots does not contain %s" % src.path, "srcs")

becomes

found  = False
for root in roots:
    if src.path.startswith(root):
        found = True
        modules.append(create_module(name = src.path[len(root):], file = src))
        break
if not found:
    fail("Module roots does not contain %s" % src.path, "srcs")

The Python is far more readable than the workaround.

@alandonovan
Copy link
Contributor

alandonovan commented Oct 1, 2020

I was about to say that the desugaring is incorrect: the found=True statement should be outside the if statement. But then I checked and found that, no, you are correct, which only proves my point: the semantics of 'else' are far from obvious, even to someone familiar with a dozen other languages. So again I say the desugared code is more readable, though obviously less concise.

Regarding significant whitespace, it may be surprising when you first see it, and it may be a cause of carelessly inserted mistakes, but its meaning is abundantly obvious.

@laurentlb
Copy link
Contributor

Your rewrite with the found boolean seems more obvious to me. This feature is rarely found in other languages and it can create confusion.

I've migrated tons of files at Google from Python to Starlark, and this else pattern was very rarely used in practice. I really don't think we need this.

@pauldraper
Copy link
Author

But then I checked and found that, no, you are correct, which only proves my point

Or rather it proves my point, that you would have desugared wrong and the original form was a more reliable to express a common sort of thing.

This feature is rarely found in other languages and it can create confusion.

And Skylark still chooses to have significant whitespaces.

It seems that presence of feature in other languages is not actually a criterion.

@laurentlb
Copy link
Contributor

Whitespace is not a feature, it's the base of the language. You might say it was a mistake and another syntax would be better, but looking like Python was a design principle (https://github.com/bazelbuild/starlark#design-principles). That's not relevant to this thread.

There's also a desire to keep the number of features low and limit the number of concepts someone needs to know in order to read the code.

Or rather it proves my point, that you would have desugared wrong

The desugared form is obvious. Read the code, it does what it says.
On the other hand, you wouldn't necessarily guess the meaning of else (Does it run if we didn't enter the first block, like in if? Actually, no).

It seems that presence of feature in other languages is not actually a criterion.

Familiarity is one of the goals.

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

No branches or pull requests

3 participants