Thread overview | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
September 17, 2020 Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
As wc -l counts, phobos has some 330 KLOC: $ wc -l $(git ls-files '*.d') | tail -1 331378 total I noticed many contributors are fond of inserting empty lines discretionarily, sometimes even in the middle of 2-5 line functions, or right after opening an "if" statement. The total number of empty lines: $ git grep '^$' $(git ls-files '*.d') | wc -l 38503 So Phobos has 11.62% empty lines in it, on average one on every 9 lines of code. I find that a bit excessive, particularly given that our coding convention uses brace-on-its-own line, which already adds a lot of vertical space. Here's the number of lines consisting of only one brace: git grep '^ *[{}] *$' **/*.d | wc -l 53126 That's 16% of the total. Combined with empty lines, we're looking at a 27.65% fluff factor. Isn't that quite a bit, even considering that documentation requires empty lines for paragraphs etc? Today's monitors favor width over height and I didn't yet get to the point of rotating my monitor for coding purposes. (It's also curved, which would make it awkward.) Would it be reasonable to curb a bit on the fluff factor? E.g. there should never be two consecutive empty lines, and code blocks shorter than x lines should have no newlines inside. |
September 17, 2020 Re: Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On 9/17/20 11:51 AM, Andrei Alexandrescu wrote:
> git grep '^ *[{}] *$' **/*.d | wc -l
This should be:
git grep '^ *[{}] *$' $(git ls-files '*.d') | wc -l
(The **/*.d syntax works only with zsh.)
|
September 17, 2020 Re: Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Thu, Sep 17, 2020 at 11:51:18AM -0400, Andrei Alexandrescu via Digitalmars-d wrote: > As wc -l counts, phobos has some 330 KLOC: > > $ wc -l $(git ls-files '*.d') | tail -1 > 331378 total > > I noticed many contributors are fond of inserting empty lines discretionarily, sometimes even in the middle of 2-5 line functions, or right after opening an "if" statement. Do you have a concrete example of this? > The total number of empty lines: > > $ git grep '^$' $(git ls-files '*.d') | wc -l > 38503 > > So Phobos has 11.62% empty lines in it, on average one on every 9 lines of code. I find that a bit excessive, particularly given that our coding convention uses brace-on-its-own line, which already adds a lot of vertical space. Here's the number of lines consisting of only one brace: > > git grep '^ *[{}] *$' **/*.d | wc -l > 53126 > > That's 16% of the total. Combined with empty lines, we're looking at a 27.65% fluff factor. Isn't that quite a bit, even considering that documentation requires empty lines for paragraphs etc? IMO, it depends. Code formatted into logically-grouped paragraphs is easier to read, esp. to quickly scan to get a quick overview of what the function does. If it's all smushed into a solid 10-line block, it would be much harder to read. But I don't think there's a fixed number of lines where the cutoff is -- it depends on what the code is trying to do, and so has to be judged on a case-by-case basis. OTOH, if there's a bunch of 1-line functions, formatting it in Phobos style with braces on separate lines would occupy 4-5 lines per function, which is rather excessive: // Too much whitespace struct MyRange(E) { @property bool empty() { return _isEmpty; } @property E front() { return _front; } void popFront() { r.popFront; } } as opposed to: // Better struct MyRange(E) { @property bool empty() { return _isEmpty; } @property E front() { return _front; } void popFront() { r.popFront; } } > Today's monitors favor width over height and I didn't yet get to the point of rotating my monitor for coding purposes. (It's also curved, which would make it awkward.) Would it be reasonable to curb a bit on the fluff factor? Sure, but why are we fussing over formatting, when there are bigger fish to fry? Like code quality issues: pushing sig constraints into static ifs when they are more suitable, merging unnecessary overloads, fixing documentation issues, etc.. IMO, removing empty lines is last on that long list of action items. > E.g. there should never be two consecutive empty lines, and code blocks shorter than x lines should have no newlines inside. You mean no empty lines? Otherwise we'd have to pack multiple statements onto 1 line, which I don't think is what you want. ;-) T -- Don't drink and derive. Alcohol and algebra don't mix. |
September 17, 2020 Re: Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
Posted in reply to H. S. Teoh | On Thursday, 17 September 2020 at 16:34:33 UTC, H. S. Teoh wrote:
> [snip]
>
> Sure, but why are we fussing over formatting, when there are bigger fish to fry? Like code quality issues: pushing sig constraints into static ifs when they are more suitable, merging unnecessary overloads, fixing documentation issues, etc.. IMO, removing empty lines is last on that long list of action items.
>
I.e., does having extra empty lines have an impact on the time it takes to compile phobos?
I don't really have a good sense of whether the time spent compiling is more related to number of lines or number of characters. An empty line with no characters or one character may not take long to parse.
|
September 17, 2020 Re: Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
Posted in reply to H. S. Teoh | On 9/17/20 12:34 PM, H. S. Teoh wrote: > On Thu, Sep 17, 2020 at 11:51:18AM -0400, Andrei Alexandrescu via Digitalmars-d wrote: >> As wc -l counts, phobos has some 330 KLOC: >> >> $ wc -l $(git ls-files '*.d') | tail -1 >> 331378 total >> >> I noticed many contributors are fond of inserting empty lines >> discretionarily, sometimes even in the middle of 2-5 line functions, >> or right after opening an "if" statement. > > Do you have a concrete example of this? Only everywhere I look. Just opened std.algorithm.iteration but really no need to cherry-pick: Four-liner unittest with a gratuitous empty line: https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L278. Also uses some vertical alignment that maintainers are going to just love. Empty line BEFORE the closing "}", that's just sloppy: https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L2267 An empty line between the cases and the default of a switch: https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L5863. The switch itself is four lines. Also: the default case uses return on the same line, the others don't. An empty line inexplicably just before the return of a four-liner. A four-liner! Who thinks this makes anything easier to read? https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L5902 An empty line in the middle of a three-line block, again before the return (what's the deal with that)? https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L6787. Three-liner unittest with an empty one added: https://github.com/dlang/phobos/blob/master/std/algorithm/iteration.d#L7004 The list goes on... (I recall being in a similar debate with a colleague, and he mentioned he separates with a newline all non-expression statements (or something to that effect). So if there's an if, a loop, an import, a return, a definition, etc. - they have newlines in between. I'd think that gets into career-limiting stuff, especially because said colleague had the air that that's really fundamental, like the theorem of structure.) This all is a bit of a bummer. I reckon it's a popular style because there seem to be a lot of code in phobos like that, but it kinda is excessive and limits the context one can put on the screen. Worse, I saw that as a side-effect there's a lot of "cheating" on the full bracing - many places that require no full braces dispense with them. However, it turns out that full-bracing is useful during maintenance (you can easily insert or remove code), whereas empty lines aren't. > Sure, but why are we fussing over formatting, when there are bigger fish > to fry? Like code quality issues: pushing sig constraints into static > ifs when they are more suitable, merging unnecessary overloads, fixing > documentation issues, etc.. IMO, removing empty lines is last on that > long list of action items. Per the Romanian saying, "til you get to God the saints will eat you up". Hard to translate. Meaning you try the big thing but the little things are going to get you first. Yah, I'm trying to do the good things while constantly getting sand in the eye because of the little things. >> E.g. there should never be two consecutive empty lines, and code >> blocks shorter than x lines should have no newlines inside. > > You mean no empty lines? Otherwise we'd have to pack multiple > statements onto 1 line, which I don't think is what you want. ;-) Not no two consecutive newlines. Two consecutive empty lines, i.e. three consecutive newlines, \n\n\n. |
September 17, 2020 Re: Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
Posted in reply to jmh530 | On 9/17/20 1:06 PM, jmh530 wrote:
> On Thursday, 17 September 2020 at 16:34:33 UTC, H. S. Teoh wrote:
>> [snip]
>>
>> Sure, but why are we fussing over formatting, when there are bigger fish to fry? Like code quality issues: pushing sig constraints into static ifs when they are more suitable, merging unnecessary overloads, fixing documentation issues, etc.. IMO, removing empty lines is last on that long list of action items.
>>
>
>
> I.e., does having extra empty lines have an impact on the time it takes to compile phobos?
>
> I don't really have a good sense of whether the time spent compiling is more related to number of lines or number of characters. An empty line with no characters or one character may not take long to parse.
This is exclusively about readability. Too much fluff means too little context in front of you.
|
September 17, 2020 Re: Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Thursday, 17 September 2020 at 17:16:42 UTC, Andrei Alexandrescu wrote:
> [snip]
>
> This is exclusively about readability. Too much fluff means too little context in front of you.
When I looked at some of those examples above, I agree that some are extraneous (which should get flagged by the autotester)...but I almost always put an empty line after imports. I find that helps with readability and even maintainability in short functions/scopes in that I don't need to add them back after adding more lines and have more trouble finding them.
|
September 17, 2020 Re: Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
Posted in reply to Andrei Alexandrescu | On Thursday, 17 September 2020 at 15:51:18 UTC, Andrei Alexandrescu wrote: > As wc -l counts, phobos has some 330 KLOC: > > $ wc -l $(git ls-files '*.d') | tail -1 > 331378 total > > I noticed many contributors are fond of inserting empty lines discretionarily, sometimes even in the middle of 2-5 line functions, or right after opening an "if" statement. The total number of empty lines: > > $ git grep '^$' $(git ls-files '*.d') | wc -l > 38503 > > So Phobos has 11.62% empty lines in it, on average one on every 9 lines of code. I find that a bit excessive, particularly given that our coding convention uses brace-on-its-own line, which already adds a lot of vertical space. Here's the number of lines consisting of only one brace: > > git grep '^ *[{}] *$' **/*.d | wc -l > 53126 > > That's 16% of the total. Combined with empty lines, we're looking at a 27.65% fluff factor. Isn't that quite a bit, even considering that documentation requires empty lines for paragraphs etc? > > Today's monitors favor width over height and I didn't yet get to the point of rotating my monitor for coding purposes. (It's also curved, which would make it awkward.) Would it be reasonable to curb a bit on the fluff factor? E.g. there should never be two consecutive empty lines, and code blocks shorter than x lines should have no newlines inside. We shouldn't have these discussions. We should just use a code formatting tool (see e.g. how successful black was in the Python world - https://github.com/psf/black) and be done with. So IMHO the only productive discussion here is how we can get dfmt (https://github.com/dlang-community/dfmt) (or a new tool) in a shape, s.t. it can be applied automatically. Alternatively, for your specific request, there was [1], but I since gave up on enforcing such style issues manually. As I mentioned, a tool should handle this task for you. [1] https://github.com/dlang-community/D-Scanner/pull/447 |
September 17, 2020 Re: Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
Posted in reply to jmh530 | On Thursday, 17 September 2020 at 17:06:12 UTC, jmh530 wrote:
>
> I.e., does having extra empty lines have an impact on the time it takes to compile phobos?
No impact at all. Not even for lexing.
|
September 17, 2020 Re: Is phobos too fluffy? | ||||
---|---|---|---|---|
| ||||
Posted in reply to jmh530 | On Thu, Sep 17, 2020 at 05:06:12PM +0000, jmh530 via Digitalmars-d wrote: [...] > I.e., does having extra empty lines have an impact on the time it takes to compile phobos? > > I don't really have a good sense of whether the time spent compiling is more related to number of lines or number of characters. An empty line with no characters or one character may not take long to parse. Lexing/parsing is the fastest part of the compilation process. The majority of compilation time (by far) is spent on semantic analysis and codegen. A couple of empty lines makes a difference so minute I daresay it's not even measurable. T -- If it tastes good, it's probably bad for you. |
Copyright © 1999-2021 by the D Language Foundation