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

Rule request: Use min() or max() instead of sorted().first or sorted().last #1932

Closed
tomquist opened this Issue Nov 6, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@tomquist
Contributor

tomquist commented Nov 6, 2017

New Issue Checklist

Rule Request

If this is a new rule request, please ignore all sections below this one, format
this issue's title as Rule Request: [Rule Name] and describe:

  1. Why should this rule be added? Share links to existing discussion about what
    the community thinks about this.
    First sorting a sequence and then just reading the first or last element is unnecessarily consuming memory and is much slower (O(n log(n))) than searching for the minimum element in a sequence (O(n)).

  2. Provide several examples of what would and wouldn't trigger violations.

    // Should trigger
    myList.sorted().first
    myList.sorted(by: { $0.description < $1.description }).first
    myList.sorted(by: >).first
    myList.map { $0 + 1 }.sorted().first
    myList.sorted(by: someFunction).first
    myList.map { $0 + 1 }.sorted { $0.description < $1.description }.first
    myList.sorted().last
    myList.sorted().last?.something()
    myList.sorted(by: { $0.description < $1.description }).last
    myList.map { $0 + 1 }.sorted().last
    myList.sorted(by: someFunction).last
    myList.map { $0 + 1 }.sorted { $0.description < $1.description }.last
    // Should not trigger
    let min = myList.min()\n
    let min = myList.min(by: { $0.description < $1.description })
    let min = myList.min(by: >)
    let min = myList.max()
    let min = myList.max(by: { $0 < $1 })
  1. Should the rule be configurable, if so what parameters should be configurable?
    Just severity.
  2. Should the rule be opt-in or enabled by default? Why?
    The rule may lead to false positives on non-sequence API where there is no min/max function. Similar to first_where and contains_over_first_not_nil this rule should be opt-in.
@jpsim

This comment has been minimized.

Show comment
Hide comment
@jpsim

jpsim Nov 8, 2017

Collaborator

Done in #1933.

Collaborator

jpsim commented Nov 8, 2017

Done in #1933.

@jpsim jpsim closed this Nov 8, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment