Name boolean arguments #2164
Name boolean arguments #2164
Conversation
What would you prefer? adaptToMemberWithArgs(tree, qual, name, mode, false, false) Or: adaptToMemberWithArgs(tree, qual, name, mode, reportAmbiguous = false, saveErrors = false)
|
Hat tip to @Alefas for the recent improvements to the IntelliJ Scala plugin that means I can now run its inspections over our codebase. |
|
would be nice to know that it doesn't add add any bytecode (i.e. that the names got eliminated at compile time) - did you check that maybe..? |
|
@lrytz No, i didn't check. Are you worried about the arguments being in a different order to the the parameters? |
|
no, i was thinking of a sanity check for the named arguments implementation, that it doesn't generate any assignment blocks if things are in order. i should add some bytecode tests for that.. |
|
@lrytz I've added a test for that. |
When named arguments correspond the the parameter declaration order, the compiler should not lift out assignments before the method call, as it would have to do for out-of-order arguments. Confirm this with a bytecode comparison test.
|
nice thanks! |
|
I think we should work harder to eliminate boolean parameters before we serve as their enablers. This patch, while unarguably better considered in isolation at a frozen moment in time, also serves to:
I think quite a few of them can be eliminated with simply better thought out structure, and of whatever is left, we should be sure they carry an excellent name before we give every call site a dependency on it. |
|
Also, I couldn't find the minor chuckles. I think that link is particular to you (for creating a pull request.) |
|
Here are the chuckles. https://github.com/scala/scala/pull/2164/files#L62L15 |
|
Ha! The only way the chuckles could have been less minor is if you had had to reorder the parameters (i.e. the comments had bitrotted.) |
|
So I don't buy the argument that we should avoid the merely good because we'll then be too lazy to go for the great. All too often, we're left with the bad. It is still straightforward to improve the parameter names / eliminate boolean parameters after the fact using the same refactoring tools. But let's be concrete and use this review to highlight sub-optimal names and a the most-wanted list for boolean parameters elimination. |
|
Looking through the list of methods taking boolean arguments, I literally did not see a single one I did not want to put on the list for methods which should be written not to take boolean arguments. That said, I'll go along with the "merely good". |
|
Superseded by #2203. |
What would you prefer?
Or:
compiler, reflect, and library have been swept.
A sanity check is included to show that using named args for clarity incurs no bytecode penalty.
Review by @paulp. Minor chuckles here: https://github.com/retronym/scala/pull/new/topic/bool-arg#L62L15