-
-
Notifications
You must be signed in to change notification settings - Fork 382
Make average line length example more concise and move it to the top #1711
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
wilzbach
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like this example. How about adding it to the example code roulette as well?
index.dd
Outdated
| lines ? cast(double) sumLength / lines : 0.0); | ||
| import std.range, std.stdio; | ||
| auto sum = 0.0; | ||
| auto count = stdin.byLine().tee!(l => sum += l.length).walkLength; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: maybe removing the parentheses here to be more in the D-style?
Is there a way to share the source code between the different examples? Or perhaps we should swap this one with the rounding one? |
|
Btw I just realized that we probably need to update the MD5 hash: https://github.com/dlang/dlang.org/blob/master/js/run-main-website.js Imho this is a really annoying approach and we get rid of this approach entirely and store the STDIN in the DOM as well. This manual hashing approach wouldn't scale anyways if we go with #1709
From // [your code here] rotation for index.html
var $examples = $('.your-code-here-extra > pre');
if ($examples.length) {
var n = Math.floor(Math.random() * ($examples.length+1));
if (n)
$('#your-code-here-default > pre').replaceWith($examples[n-1]);
}So in theory just adding another $(EXTRA_EXAMPLE should work...
I would leave it in the roulette for now as we only have two examples at the moment. |
|
|
So what's left for me to do? Copy & paste the example to the top? |
Yep, but better do this as a separate PR, so that this one isn't blocked |
Once you have rebased your PR, you should be able to do this :) |
|
@wilzbach done. @CyberShadow @aG0aep6G what do you think? |
And also make it the default. The primary motivation is that we need to have a simple example for first time visitors.
|
|
||
| auto sum = 0.0; | ||
| auto count = stdin.byLine | ||
| .tee!(l => sum += l.length).walkLength; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even shorter would be:
size_t count = 0;
double sum = stdin.byLine.tee!(l => count++).sum;but I guess that demonstrates fewer range features.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that would compile, unless you add a map!(l => l.length) between the tee and the sum.
/d912/f975.d(7): Error: template std.algorithm.iteration.sum cannot deduce function from argument types !()(Result), candidates are:
/opt/compilers/dmd2/include/std/algorithm/iteration.d(4663): std.algorithm.iteration.sum(R)(R r) if (isInputRange!R && !isInfinite!R && is(typeof(r.front + r.front)))
/opt/compilers/dmd2/include/std/algorithm/iteration.d(4674): std.algorithm.iteration.sum(R, E)(R r, E seed) if (isInputRange!R && !isInfinite!R && is(typeof(seed = seed + r.front)))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, you're right :)
|
|
Ping @wilzbach @CyberShadow is this good to go? |
|
Thanks @CyberShadow! |
The primary motivation is that we need to have a simple example for first time visitors.
For comparison: before and after.