dart-lang / site-www

The source for the Dart website.
https://dart.dev
Other
970 stars 702 forks source link

Consider adding documentation about huge methods #5727

Open alexmarkov opened 2 years ago

alexmarkov commented 2 years ago

In order to compile extremely huge methods compiler may spend a lot of time and memory, and may eventually run out of memory. Also, compiler may compile such methods with certain optimizations disabled or reduced (such as inlining). Huge methods are also harder to reason about and maintain.

So we should suggest users to avoid writing (or generating) too large methods, maybe in "Effective Dart". We can suggest extracting common repeated patterns or abstractions out of these methods and splitting them into multiple methods.

Related: https://github.com/dart-lang/sdk/issues/48495.

parlough commented 2 years ago

This seems like a great thing to document even just in terms of maintenance and understanding like you mentioned.

Perhaps good to include in the Effective Dart: Usage - Functions section.

\cc @munificent What are your thoughts on including something along these lines or do you think this documentation belongs elsewhere?

bernaferrari commented 2 years ago

How do you define a "huge method"?

alexmarkov commented 2 years ago

I don't think we need to have a strict definition of huge methods in the documentation. It could be just a recommendation to avoid unreasonably large methods and avoid repeated code.

In my opinion, in order to improve source code readability and maintainability, developers should avoid methods larger than 300-500 of lines of reasonably formatted code (not counting comments and blank lines), because it's becomes increasingly harder to follow the logic of larger methods. This is very subjective - there are people who don't like methods larger than a couple of screens and there are people who are okay with reading and understanding 1K LOC methods.

The problems like large binary code size, slow compilation and high memory usage during compilation will appear on much larger methods (think of at least a few thousands of lines) and highly depend on the actual code (so the number of lines is not a very good indicator here - the contents of the code is much more important).

Another thing to consider is that the recommendations in the documentation should probably agree to the thresholds chosen for the implementation of the corresponding analyzer lint (dart-lang/sdk#58679).

munificent commented 2 years ago

\cc @munificent What are your thoughts on including something along these lines or do you think this documentation belongs elsewhere?

I'm always somewhat hesitant to add more to "Effective Dart" because it's already quite large and the bigger it is, the greater the chance users simply won't read it.

I try to focus on guidelines that are useful in that it's (1) a problem and (2) users do seem to do it in the wild fairly frequently. Huge methods are (1) but I can't say I often see them in the wild, so I'm disinclined to add a rule telling people not to do a thing that almost all of them are already not doing. Feels a bit like spending legislative time passing a law outlawing kinkajous in water parks.

atsansone commented 7 months ago

@munificent : Do you think we should close this issue?

munificent commented 6 months ago

Yeah, I think so.