Skip to content

proposal: sort_public_methods_before_private_methods #58910

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

Closed
zellidev0 opened this issue Oct 28, 2022 · 4 comments
Closed

proposal: sort_public_methods_before_private_methods #58910

zellidev0 opened this issue Oct 28, 2022 · 4 comments
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-lint-proposal linter-status-pending

Comments

@zellidev0
Copy link

zellidev0 commented Oct 28, 2022

sort_public_methods_before_private_methods

Description

Define public methods before private methods.

Details

DO define public methods before private methods.

This makes the code uniform across multiple classes and it makes it faster to find specific methods in a class.
Because public methods are the interface to the class they are (often) more important and should come before private methods.

This would be my first approach to have a defined order for different parts in a class.
Flutter style guide ordering of class members

Kind

Style

Good Examples

class A {
  int a() => 0;
  int c() => 0;
  int _b() => 0;
}

Bad Examples

class A {
  int a() => 0;
  int _b() => 0;
  int c() => 0;
}
@bwilkerson
Copy link
Member

I made a couple of comments on the PR, but we should really discuss the semantics (and desirability) of the lint before getting too far into the implementation issues.

Do we want to have a rule that enforces part of the sort order, or would it be better to have a rule that enforces a more complete sort order (such as the one produced by the "Sort Members" feature)? We do already have a couple of rules about the order of constructors, but we need to consider interactions between all such rules, and the more rules we have the harder it will be to manage the interactions. We could, for example, have a rule to enforce that "Sort Members" has been run over the code.

If we do want a more limited rule like this, how does this relate to different kinds of members? The "Sort Members" feature, for example produces the following order:

  • public static fields
  • private static fields
  • public instance fields
  • private instance fields
  • public constructors
  • private constructors
  • public instance methods
  • private instance methods
  • public static methods
  • private static methods

Public members of a given kind are always before private members of the same kind, but the kinds are kept separate. Does this rule propose that private members of any kind always be after public members of every kind?

@srawlins
Copy link
Member

Possible duplicate of #58283

@srawlins
Copy link
Member

We will need some strong justification in order to incorporate this proposed rule. We are only accepting new style-based lint rules which are promoted by the Effective Dart style guide, or the Flutter style guide. I don't believe either recommends putting public methods before private ones.

I typically place private methods near their use, for example. Google's Java Style Guide recommends a logical order, but doesn't call out public-before-private, as it really is on a class-by-class basis. Our internal Java Style Guide (OK I have no idea why they're different) spells out the style I use:

Some of these members feel like "footnotes" and some feel like "appendices". If a member exists only to support a single other method, put it just below that method ("footnote" style). But if it is (or feels like) a more general helper, then put it toward the end of the class ("appendix" style).

@srawlins
Copy link
Member

I'll close this for now. If we have a top-priority style guide which recommends an ordering of members, we can re-consider this rule.

@devoncarew devoncarew added devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. labels Nov 19, 2024
@devoncarew devoncarew transferred this issue from dart-archive/linter Nov 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devexp-linter Issues with the analyzer's support for the linter package legacy-area-analyzer Use area-devexp instead. linter-lint-proposal linter-status-pending
Projects
None yet
Development

No branches or pull requests

4 participants