Use slices and maps more where appropriate
#2306
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kolyshkin, rhatdan The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
cmd/containers-storage/main.go
Outdated
| } | ||
| cmd := args[0] | ||
|
|
||
| for _, command := range commands { |
There was a problem hiding this comment.
Non-blocking: This works, and it is an improvement already, but processing commands in a loop is not quite what we mean.
Consider
i := slices.IndexFunc(commands, func(c) { slices.Contains(c.names, cmd) })
if i == -1 { /* fail */ }
command := commands[i]
// straight-line top-level code processing the command we foundThere was a problem hiding this comment.
You are right, this looks weird.
Rewrote (I left for range as it seems more readable than slices.IndexFunc in this particular case).
...by using maps.Clone if possible. Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
Signed-off-by: Kir Kolyshkin <[email protected]>
Simplify the code that finds the command. While at it, rename cmd to name to avoid using cmd and command together. Best reviewed with --ignore-all-space. Signed-off-by: Kir Kolyshkin <[email protected]>
| fmt.Printf("%s: unrecognized command.\n", name) | ||
| os.Exit(1) | ||
| return nil // To satisfy linters. |
There was a problem hiding this comment.
(Absolutely non-blocking: Aesthetically, I’d prefer return nil here, and having os.Exit at the top level, but this works perfectly fine.)
|
/lgtm |
3 participants
Please see individual commits for details.