-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
Python style list comprehensions for gdscript #15222
Conversation
modules/gdscript/gdscript_parser.cpp
Outdated
@@ -731,6 +731,7 @@ GDScriptParser::Node *GDScriptParser::_parse_expression(Node *p_parent, bool p_s | |||
|
|||
ArrayNode *arr = alloc_node<ArrayNode>(); | |||
bool expecting_comma = false; | |||
expr = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used spaces instead of tabs
modules/gdscript/gdscript_parser.cpp
Outdated
|
||
expr = cn; | ||
break; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used spaces instead of tabs with this block
modules/gdscript/gdscript_parser.cpp
Outdated
@@ -767,7 +828,8 @@ GDScriptParser::Node *GDScriptParser::_parse_expression(Node *p_parent, bool p_s | |||
} | |||
} | |||
|
|||
expr = arr; | |||
if (!expr) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
used spaces instead of tabs
6031378
to
33f64dc
Compare
33f64dc
to
4a031d0
Compare
This saves one lookup to "append" per item and preallocates the array to the final size in simple cases, both making comprehension lists build 40% faster than through a regular loop
Thanks a lot for your contribution! Moving this PR to the 3.1 milestone, to be reviewed once the release freeze is lifted. It could eventually be cherry-picked for a future 3.0.1 maintenance release if it doesn't change the user-facing APIs and doesn't compromise the engine's stability. |
I came here looking for a way of doing #17268 but I like this suggestion more. Would love to see this. IMO lambdas might make gdscript too complex for beginners? But this is a really good compromise. |
We've discussed this on IRC with many core developers, and the general feeling is that the syntax of list comprehensions makes code quite hard to read, and goes against the design principles of GDScript to have a straightfoward and easy to read syntax. Most use cases for list comprehensions can easily be replaced with one (or nested) for loops, which are a lot more readable even if a couple lines more, so we don't see a real use case that would warrant the syntactic complexity. See discussion on http://godot.eska.me/irc-logs/devel/2018-06-20.log from 22:10. |
I’d like to contest the “unreadable” argument too. List comprehensions are actually more readable to me. Their syntax is very much like the syntax of natural English language:
Which in almost-code translates to this (still English-like):
The above sentence is eminently readable to me. I sincerely cannot understand why this would be a difficult-to-read sentence. I can actually feel more cognitive load when reading a regular for-loop compared to reading a list comprehension. It starts to diverge from English syntax with nested comprehensions, where the “English” would be:
Which in code would be:
— which I would argue is still not any more difficult to read than a nested for-loop. I am of course just speaking for myself, but then again, the detractors of list comprehensions are also just speaking for themselves and extrapolating their experience onto other people. I'd like to offer the argument that if you read a list comprehension as if it were a natural English sentence (just missing a few bits of grammar), it becomes quite easy to understand. Referencing #4716 where some people have also made some good arguments for list comprehension. |
since when is this a design metric? this is a programming language, not something you will write educational papers with. and also that argument is completely wrong as list comprehensions are more readable than three thousand nested for loops. you cant tell me it is easier for you to read this:
than this:
|
Since this repository is now used for bug reports only, feature proposals should now be discussed on the Godot proposals repository. |
See #4716
No optimizations for preallocating size yet.