On 9/17/20 1:49 PM, jmh530 wrote:
> 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 was all in favor of that, until I had to refactor a dozen two-liners consisting of... let me paste some code:

auto put(ref GGPlotD gg, GeomBar def) {
    import ggplot.backend.ggplot.geom_bar : geomBar;

    return gg.put(geomBar(def));
}

And so it goes for pages.

There is something to be said about a guideline versus a mindless dogma. Things like enum strings, each, and autodecoding are more of the latter.

On Thursday, 17 September 2020 at 17:49:39 UTC, Seb wrote:

> 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

What would be nice is if we didn't have Github shouting at us because of whitespace errors. We're not using Python after all.

On 9/17/20 1:49 PM, Seb wrote:
> 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

Great point. We have uncrustify working passably well at Symmetry, making them available for phobos is a matter of putting in the time.

On Thu, Sep 17, 2020 at 02:15:32PM -0400, Andrei Alexandrescu via Digitalmars-d wrote:
> On 9/17/20 1:49 PM, jmh530 wrote:
[...]
> > 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 was all in favor of that, until I had to refactor a dozen two-liners consisting of... let me paste some code:
> 
> auto put(ref GGPlotD gg, GeomBar def) {
>     import ggplot.backend.ggplot.geom_bar : geomBar;
> 
>     return gg.put(geomBar(def));
> }

Yeah, when it's just 2 lines like these, a blank line is just an eyesore that serves no purpose.  I say we kill 'em.


> And so it goes for pages.
> 
> There is something to be said about a guideline versus a mindless dogma.  Things like enum strings, each, and autodecoding are more of the latter.

I'm on the fence about `each`.  I totally agree that the implementation
is a bit overblown -- do we *really* need to support static arrays,
opApply iterables, iteration indices, and all of that jazz?  Whenever we
end up with a combinatorial explosion of (sig constraint helper
templates, or any kind of code construct, really), it tells me that we
have failed to separate different concerns properly. Instead of writing
O(2^N) cases explicitly, at the most it ought to be O(N) cases, or,
better yet, O(1) (i.e., push tangential concerns out of the
implementation to the caller (or someone else)).

OTOH, I see why it might sometimes be a handy thing to have: if you have a long UFCS chain, it can be an annoyance to have to wrap the entire block with a `foreach (x; a.b.c.d.e.f.g./*...*/.z)` instead of being able to just tack on `.each!(...)` at the end.  However, I'd say in this case .each really should only support ranges, since UFCS chains are the primary use case, and leave the other stuff like static arrays (not even a range by current definitions), opApply-iterables, and AA's alone.

Of course, once you take all of that other fluff out, the raison d'etre of .each becomes a little shaky. :-P  It's probably pushing it, but I'm tempted to suggest that perhaps this is one of those opportunities Andrei was talking about to improve the language: what if the language equates:

	iterable.foreach(i => { ... });

with

	foreach (i; iterable) { ... }

?  I.e., the former lowers to the latter.  This seems to be a natural (logical?) next step in the spirit of UFCS, except applied to built-in loop constructs.

Then there would be no need of .each, and UFCS functional-style code would be nicely unified with the good ole traditional loops.  And it doesn't even need further compiler complications, being merely a simple syntactic rewrite.

(Or perhaps this is illogical and just wayyyy out there... let the
rotten fruit fly. :-D)


T

-- 
Lottery: tax on the stupid. -- Slashdotter

On Thursday, 17 September 2020 at 18:15:32 UTC, Andrei Alexandrescu wrote:
> [snip]
>
> I was all in favor of that, until I had to refactor a dozen two-liners consisting of... let me paste some code:
>
> auto put(ref GGPlotD gg, GeomBar def) {
>     import ggplot.backend.ggplot.geom_bar : geomBar;
>
>     return gg.put(geomBar(def));
> }
>
> And so it goes for pages.
[snip]

Oh I probably have lots of code that does stuff like that (even more commonly in unittests).

But I also agree with the points above on dfmt and having this done automatically.

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.

Good that someone from the core team pays attention to these things. We should encourage the use of dfmt for such things, come up with a standard .editorconfig and commit that to the repo. Vim, emacs, VSCode - pretty much the majority of the editors support dfmt + .editorconfig. The more dogfooding, the better it is. That's not happening at the moment.

druntime uses different style from Phobos as well.

I wish all the community projects use a common D-style (even the one recommended officially). But I understand where that discussion will lead to.

On Thursday, September 17, 2020 11:49:05 AM MDT jmh530 via Digitalmars-d wrote:
> 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.

I do the same. Similarly, I usually put blank lines after declarations, because that makes the code easier to read, but I'm also somewhat flexible about it, since following strict rules about some of that stuff can certainly result in way too many blank lines in some code. About the only time I'll consider not putting a blank line after an import statement is when the function is otherwise a one-liner, but even then, I'm likely to prefer having the blank line (which Andrei clearly wouldn't like).

Either way, while I won't claim that Phobos' code always does a good job with where it has blank lines (and there certainly are plenty of times that I haven't like code formatting in Phobos), I haven't personally found that the number of blank lines in Phobos causes problems with being able to see code on a screen even on my laptop, and in my experience, trying to minimize blank lines often makes code harder to read. It's a very subjective thing though and is going to vary from person to person. Using a code formatter would make it so that you don't have to argue about it in PRs, but it also tends to result in uglier code. Personally, I'd prefer that we just not be picky about it, but if we're going to be, then ultimately, using a code formatter is probably the way to go.

That being said, unless the code formatter is smart enough to deal with very small functions differently, removing blank lines after something like import statements in order to reduce the number of blank lines in small functions will definitely make longer functions harder to read - especially when we're doing a lot with local import statements. You can get quite a list of those in a row, in which case, I think that it's pretty bad not have to have a blank line after it. I don't know how smart any code formatting tools we currently have at our disposal are though, since I generally avoid them.

- Jonathan M Davis

On Thursday, 17 September 2020 at 18:44:09 UTC, H. S. Teoh wrote:
> Of course, once you take all of that other fluff out, the raison d'etre of .each becomes a little shaky. :-P  It's probably pushing it, but I'm tempted to suggest that perhaps this is one of those opportunities Andrei was talking about to improve the language: what if the language equates:
>
> 	iterable.foreach(i => { ... });
>
> with
>
> 	foreach (i; iterable) { ... }
>
> ?  I.e., the former lowers to the latter.  This seems to be a natural (logical?) next step in the spirit of UFCS, except applied to built-in loop constructs.

This seems totally backwards to me. Why implement something as a language feature when we can already do it with library code?

Also, "foreach but it works in range pipelines" is absolutely a worthwhile reason on its own to keep .each around. I understand some people don't want "trivial" stuff in Phobos, but there is a real value to convenience. (Not to mention that even seemingly-trivial generic code can have edge-cases that take some work to get right.)

To give a negative example: C++'s <random> library makes essentially no concessions to convenience, and as a result is gratuitously annoying to use. This blog post by Martin Hořeňovský goes into some of the details:

https://codingnest.com/generating-random-numbers-using-c-standard-library-the-solutions/

On Thu, Sep 17, 2020 at 08:36:38PM +0000, Paul Backus via Digitalmars-d wrote:
> On Thursday, 17 September 2020 at 18:44:09 UTC, H. S. Teoh wrote:
[...]
> > what if the language equates:
> > 
> > 	iterable.foreach(i => { ... });
> > 
> > with
> > 
> > 	foreach (i; iterable) { ... }
[...]
> This seems totally backwards to me. Why implement something as a language feature when we can already do it with library code?

Oh well, it was worth a shot. :-P


> Also, "foreach but it works in range pipelines" is absolutely a worthwhile reason on its own to keep .each around. I understand some people don't want "trivial" stuff in Phobos, but there is a real value to convenience. (Not to mention that even seemingly-trivial generic code can have edge-cases that take some work to get right.)

Convenience, esp. for small wrappers like .each, is determined by frequency of use.  Is there an easy way to count the number of uses of .each in public D projects?  It'd be good to have some data to back up our decisions, otherwise we're just sailing blind based on pure speculation.


> To give a negative example: C++'s <random> library makes essentially no concessions to convenience, and as a result is gratuitously annoying to use.  This blog post by Martin Hořeňovský goes into some of the details:
> 
> https://codingnest.com/generating-random-numbers-using-c-standard-library-the-solutions/

Oy, don't get me started on C++... Once, I had an old C++ project where I identified a particular performance bottleneck that could be improved by using hashtables.  Should be easy, right?  Esp. since C++11 finally(!) has hashtables in the standard library.  Long story short, yeah there are hashtables, all right, but the interface is so klunky to use and the defaults so unhelpful that it would require major code refactoring just to get it to work (not to mention an already significant amount of code cleanup to make the original C++98 code compile with C++11 -- all just to get hashtables!).

After fighting with it for more hours than I was prepared to spend on what I thought would be a "trivial" change, I threw in the towel... and later decided to rewrite the entire code in D -- and have been very happy with this latter decision ever since!  One of the major sources of happiness was the way D has AA's as part of the language, and using structs as AA keys Just Worked(tm) out-of-the-box.  Whatever warts AA may have had (or still have), it's still orders of magnitude better than the totally-unfriendly C++ API.


T

-- 
ASCII stupid question, getty stupid ANSI.

On 17.09.20 18:34, H. S. Teoh wrote:
> 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.

(The discussed aspects of the Phobos code style make it annoying to contribute.)