Remove most of the use of else after return from JS library code. NFC
#17450
Conversation
|
The code size benefits don't really show here because we don't include most of these JS libraries as part of our codesize tests (perhaps we should measure the size of |
src/library_egl.js
Outdated
| @@ -645,12 +643,12 @@ var LibraryEGL = { | |||
| eglGetCurrentSurface: function(readdraw) { | |||
| if (readdraw == 0x305A /* EGL_READ */) { | |||
| return EGL.currentReadSurface; | |||
| } else if (readdraw == 0x3059 /* EGL_DRAW */) { | |||
| } | |||
| if (readdraw == 0x3059 /* EGL_DRAW */) { | |||
There was a problem hiding this comment.
I find else-if actually easier to read for a thing like this, where it's a series of ifs on the same thing. Do you disagree?
There was a problem hiding this comment.
Maybe marginally easier to read in some cases. But I think that benefits outweigh the costs:
- less code
- less indentation
- Single rule for all cases ("else after return is always wrong") keeps things simple.
Since tidying up this stuff in various places I've trained my self to see else after return a code smell, so for me the old code now looks wrong :)
src/library_egl.js
Outdated
| @@ -645,12 +643,12 @@ var LibraryEGL = { | |||
| eglGetCurrentSurface: function(readdraw) { | |||
| if (readdraw == 0x305A /* EGL_READ */) { | |||
| return EGL.currentReadSurface; | |||
| } else if (readdraw == 0x3059 /* EGL_DRAW */) { | |||
| } | |||
| if (readdraw == 0x3059 /* EGL_DRAW */) { | |||
There was a problem hiding this comment.
Actually, I'm happy to revert those 3 cases for now, if you are OK with the rest of the change landing?
There was a problem hiding this comment.
I don't feel strongly, I guess, if you prefer the other form.
There was a problem hiding this comment.
I reverted the three places you mentioned..
sbc100
force-pushed
the
UTF8ArrayToString
branch
from
9fd6dc5 to
9c9398d
Compare
sbc100
force-pushed
the
else_after_return
branch
from
f5d9b1e to
ccc8f36
Compare
This is small code size win but it looks like it only effects the unoptimized (non-closure) build. Still good coding style anyway. I noticed this while working on #17449 and wrong a little command line to try to find all these: ``` $ pcregrep -n -M 'return[^\n]*;\n\s*}\s*else\s*{' src/*.js ```
sbc100
force-pushed
the
else_after_return
branch
from
ccc8f36 to
f05f835
Compare
This is small code size win but it looks like it only effects the
unoptimized (non-closure) build. Still good coding style anyway.
I noticed this while working on #17449 and wrong a little command line
to try to find all these: