D component programming is a joke (Was: Re: Component programming)

[H. S. Teoh]

On Sun, Aug 04, 2013 at 05:02:05AM +0200, bearophile wrote:
[...]
> A bit improved chunkBy could go in Phobos.

Yeah I'll look into that sometime. It'll definitely be a useful thing to
have, I think.


> ---------------
> 
> >For our purposes, though, we can't just do this in a loop, because
> >it has to interface with the other components, which do not have a
> >matching structure to a loop over dates.<
> 
> It's just because D doesn't yet have a yield designed like C#.
> "yield" for coroutines is a very nice kind of glue.

Don't fibres already have yield()?


> ---------------
> 
> auto datesInYear(int year) {
>     return Date(year, 1, 1)
>         .recurrence!((a,n) => a[n-1] + dur!"days"(1))
>         .until!(a => a.year > year);
> }
> 
> 
> ===>
> 
> 
> auto datesInYear(in uint year) pure /*nothrow*/

I think "in uint" is redundant. Uints don't need to be const.

The Date ctor isn't nothrow, unfortunately.

Also, std.datetime docs explicitly state that years can be negative (in
which case they represent B.C..


> in {
>     assert(year > 1900);

Good idea, I should be using contracts. :)

But in this case, there's no good reason to restrict it to >1900,
because std.datetime can handle years past 0 AD.


> } body {
>     return Date(year, 1, 1)
>            .recurrence!((a, n) => a[n - 1] + dur!"days"(1))
>            .until!(d => d.year > year);
> }
> 
> 
> I suggest to align the dots vertically like that. And generally
> _all_ variables/arguments that don't need to mutate should be const
> or immutable (or enum), unless holes in Phobos or in the type system
> or other factors prevent you to do it.

Hmm. OK, but I still think "in uint" is redundant.


> ---------------
> 
> return chunkBy!"a.month()"(dates);
> 
> ===>
> 
> return dates.chunkBy!q{ a.month };

I don't really like using q{} here. But the UFCS thing probably looks
better, considering the rest of the code.


> ---------------
> 
> byWeek() is not so simple. Most of its code is boilerplate code.
> Perhaps using a "yield" it becomes simpler.

I think Phobos needs better primitives for constructing chunked ranges.
:)  I was thinking if there's a common core that can be factored out of
chunkBy and byWeek, to make a generic range chunking function that can
handle both.


> ---------------
> 
> string spaces(size_t n) {
>     return repeat(' ').take(n).array.to!string;
> }
> 
> ===>
> 
> string spaces(in size_t n) pure nothrow {
>     return std.array.replicate(" ", n);
> }

Thanks! I didn't know about std.array.replicate. :)


> Currently in programs that import both std.range and std.array you
> have to qualify the module for replicate.
> 
> In Python this is just:
> 
> ' ' * n

The closest I ever got to that in D was to define:

	string x(string s, size_t n) { ... }

so you could write " ".x(10) as a visual approximation to " "*10.  But
this is not a good idea for other reasons ("x" is too confusing a name
and likely to clash with other symbols).


> ---------------
> 
> auto buf = appender!string();
> 
> Perhaps this suffices:
> 
> appender!string buf;

Actually, that doesn't compile.


> ---------------
> 
> string[] days = map!((Date d) => " %2d".format(d.day))(r.front)
>                 .array;
> 
> (not tested) ==>
> 
> const days = r.front.map!(d => " %2d".format(d.day)).array;

I wanted to make the return type explicit for the reader's benefit.


[...]
> If you put the days inside buf, do you really need to turn days into
> an array with array()?

The reason is actually so that I can assert on its length. Otherwise
you're right, I could just put it directly into buf.


[...]
> ---------------
> 
> If not already present this array should go in std.datetime or
> core.time:
> 
>     static immutable string[] monthNames = [
>         "January", "February", "March", "April", "May", "June",
>         "July", "August", "September", "October", "November",
> "December"
>     ];

Well, once we have a proper i18n module...


> ---------------
> 
> return to!string(spaces(before) ~ name ~ spaces(after));
> 
> ==> (untested)
> 
> return text(before.spaces, name, after.spaces);
> 
> Or maybe even just (untested):
> 
> return before.spaces ~ name ~ after.spaces;

You're right, there's no need to call to!string. I think that was left
over from older code when some of the pieces were char[].


> ---------------
> 
> auto formatMonth(Range)(Range monthDays)
>     if (isInputRange!Range && is(ElementType!Range == Date))
> {
>     assert(!monthDays.empty);
>     assert(monthDays.front.day == 1);
> 
>     return chain(
>         [ monthTitle(monthDays.front.month) ],
>         monthDays.byWeek().formatWeek());
> }
> 
> ===> (untested)
> 
> auto formatMonth(R)(R monthDays)
>     if (isInputRange!R && is(ElementType!R == Date))
> in {
>     assert(!monthDays.empty);
>     assert(monthDays.front.day == 1);
> } body {
>     return [monthDays.front.month.monthTitle]
>            .chain(monthDays.byWeek.formatWeek);

I don't like overusing UFCS when it makes the code harder to read.


> }
> 
> 
> Generally I suggest to use pre- and post conditions.

Good idea. We should use contracts more in D... in part so that there's
enough code out there to pressure people into fixing DbC-related issues.
;-)


> ---------------
> 
> return months.map!((month) => month.formatMonth());
> 
> ===> (untested)
> 
> return months.map!formatMonth;

Good idea!


> ---------------
> 
> .map!((r) =>
> 
> ===>
> 
> .map!(r =>

Heh, I didn't know you could omit the parens there!


> ---------------
> 
> int year = to!int(args[1]);
> 
> ===>
> 
> int year = args[1].to!int;

I'm on the fence about this one. I still think to!int(args[1]) reads
better. But your version is not bad, either.


> ---------------
> 
> On Rosettacode there is a shorter calendar:
> http://rosettacode.org/wiki/Calendar#D
> 
> If you want we can put, as second D entry, your calendar code
> (without unittests) in that page too.
[...]

Well, I didn't write my version to be short. :)  The goal was to use it
as an example of the kind of component-style programming Walter was
talking about.

As for putting it up on Rosettacode, sure, go ahead. :) Just make sure
you test the result first, though. Some of your suggestions actually
don't compile.

I'm going to incorporate some of your suggestions in the code and update
the article.  Thanks for the feedback!


T

-- 
It is widely believed that reinventing the wheel is a waste of time; but
I disagree: without wheel reinventers, we would be still be stuck with
wooden horse-cart wheels.