Jump to page: 1 28  
Page
Thread overview
Is phobos too fluffy?
Sep 17, 2020
H. S. Teoh
Sep 17, 2020
jmh530
Sep 17, 2020
jmh530
Sep 17, 2020
H. S. Teoh
Sep 17, 2020
Paul Backus
Sep 17, 2020
H. S. Teoh
Sep 17, 2020
jmh530
Sep 21, 2020
Abdulhaq
Sep 17, 2020
Jonathan M Davis
Sep 17, 2020
Seb
Sep 17, 2020
H. S. Teoh
Sep 17, 2020
Timon Gehr
Sep 18, 2020
Jacob Carlborg
Sep 17, 2020
Seb
Sep 17, 2020
bachmeier
Sep 18, 2020
Paolo Invernizzi
Sep 17, 2020
Arun
Sep 18, 2020
Andrej Mitrovic
Sep 19, 2020
Martin
Sep 19, 2020
Martin
Sep 18, 2020
Imperatorn
Sep 18, 2020
mw
Sep 18, 2020
Imperatorn
Sep 18, 2020
H. S. Teoh
Sep 19, 2020
Jonathan M Davis
Sep 19, 2020
Imperatorn
Sep 19, 2020
Jonathan M Davis
Sep 19, 2020
Imperatorn
Sep 19, 2020
Patrick Schluter
Sep 19, 2020
Russel Winder
Sep 19, 2020
Imperatorn
Sep 19, 2020
Russel Winder
Sep 19, 2020
Imperatorn
Sep 19, 2020
Russel Winder
Sep 20, 2020
Imperatorn
Sep 19, 2020
Imperatorn
Sep 19, 2020
data pulverizer
Sep 19, 2020
data pulverizer
Sep 19, 2020
Uknown
Sep 19, 2020
Daniel N
Sep 19, 2020
Patrick Schluter
Sep 20, 2020
wjoe
Sep 20, 2020
matheus
Sep 22, 2020
wjoe
Sep 20, 2020
wjoe
Sep 20, 2020
Paul Backus
Sep 20, 2020
Adam D. Ruppe
Sep 20, 2020
Adam D. Ruppe
Sep 20, 2020
mate
Sep 20, 2020
Adam D. Ruppe
Sep 20, 2020
mate
Sep 20, 2020
Adam D. Ruppe
Sep 21, 2020
mate
Sep 21, 2020
DlangUser38
Sep 21, 2020
mate
Sep 21, 2020
DlangUser38
September 17, 2020
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
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
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
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
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
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
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
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
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
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.
« First   ‹ Prev
1 2 3 4 5 6 7 8