Is phobos too fluffy?

[H. S. Teoh]

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.