Skip to content

syntax: replace if-else chains with expr switch stmt - #254

Merged
mvdan merged 1 commit into
mvdan:masterfrom
quasilyte:go-critic/elseif
Jun 23, 2018
Merged

syntax: replace if-else chains with expr switch stmt#254
mvdan merged 1 commit into
mvdan:masterfrom
quasilyte:go-critic/elseif

Conversation

@quasilyte

Copy link
Copy Markdown
Contributor

Usually this form is considered more idiomatic.
I've left if-else chains with init statements untouched
to less obvious changes.

Found with elseif check from go-critic linter.

Usually this form is considered more idiomatic.
I've left if-else chains with init statements untouched
to less obvious changes.

Found with `elseif` check from go-critic linter.

@mvdan mvdan left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check that the benchmarks don't get affected by this. Especially the parser, as touching the lexer code can lead to noticeable slowdowns.

@quasilyte

Copy link
Copy Markdown
Contributor Author

Sure thing.

name     old time/op  new time/op  delta
Parse-8  42.4µs ± 1%  42.2µs ± 1%  -0.57%  (p=0.011 n=9+9)
Print-8  10.3µs ± 0%  10.2µs ± 0%    ~     (p=0.150 n=9+10)

Seems there is hardly any changes at all (-0.57% is a noise here)

@mvdan

mvdan commented Jun 23, 2018

Copy link
Copy Markdown
Owner

Great, thanks. I presumed so, but you never know.

@mvdan
mvdan merged commit ca7561f into mvdan:master Jun 23, 2018
qiangli pushed a commit to qiangli/sh that referenced this pull request Jun 25, 2026
[weave-context]
issue: mvdan#254 sh: sh-func + arith ROOT-CAUSE (codex)
files: interp/api.go, interp/issue254_fidelity_test.go, interp/runner.go, interp/vars.go
commits-ahead: 0
verify: exit=0
qiangli added a commit to qiangli/sh that referenced this pull request Jun 25, 2026
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

Successfully merging this pull request may close these issues.

2 participants