A new version of "Vampire number", do you like it?

http://codepad.org/DaVxWpoA

Bye,
bearophile

On 24-3-2013 21:02, bearophile wrote:
> Some comments about the recently created "Vampire number" task in Rosettacode:
>
> The version I have modified:
> http://rosettacode.org/mw/index.php?title=Vampire_number&diff=154069&oldid=154068
>
>
> Fwend has reverted most of my changes:
> http://rosettacode.org/wiki/Vampire_number#D
>
> ------------
>
> The rationale for some of my changes:
>
> - main at the bottom. Giving a predicable order to the parts of the program is good.

Either way is fine with me.
 
>  This how all the other entries are made.
> - Removal of "if (auto factors = ...)": coding standards suggest to avoid mixing conditional tests with actions. Keeping code more tidy is safer.

I think it's convenient, it checks for null or empty, I don't find it confusing at all.

> - Removal of complex for loops "for (long n, count; n < long.max && count < 25; n++)": it's better to keep the loop semantics simpler.
> It's better for both the optimization and for the readability and keeping the code in time.

I agree that complex for loops should be avoided. I don't think this loop
is complex though, it's very simple.

(...)

With regards to immutablity: I do find it useful, but it creates an anormous amount of
code clutter. So I use it only where it's really important. For instance we have f1 and
f2, they are declared const, and then only two lines later the constness has to be cast
away again. I find that a bit over the top.

> - "if (digits.length % 2)" instead of "if (digits.length & 1)": for the compiler they are exactly the same, because
>  the value is unsigned. And using a modulus is more clear here. We are discussing about parity, not about bits.

That is a matter of opinion. If I see "% 2", my first thought is: we're checking for even... then I realize the
"== 0" is missing.

> - Annotations like "pairs ~= [i, q]; // Heap-allocated pair.":

The annotation got accidentally deleted, sorry.

> Another thing: the problem asks to return the results for 16758243290880, 24959017348650, 14593825548650,
>while your version of the program doesn't even show the last numbers. This is bad. The Python entry shows
>all three of them.

Actually the task says: "Check if the following numbers are Vampire numbers and, if so, print them and their fangs".
So you should only print it, if it is a Vampire number. I wrote the task myself, so I should know.

> A new version of "Vampire number", do you like it?
> http://codepad.org/DaVxWpoA

It's fine with me. I'm glad we got rid of the ugly cast.

Jos van Uden:

> I think it's convenient, it checks for null or empty, I don't find it confusing at all.

I agree it's shorter and it looks handy. But one of its problems is that in D there are arrays with length zero that aren't null:


import std.stdio;
int[] foo() {
    auto a = [1];
    return a[0..0];
}
void main() {
    if (auto data = foo()) {
        writeln("here");
    }
}


In the vampire code this doesn't happen because you return null (that is represented by two zero words), but in general unlike Python in D the safe and idiomatic way to test for array emptiness is to use std.array.empty. Otherwise you risk having problems.


> With regards to immutablity: I do find it useful, but it creates an anormous amount of
> code clutter. So I use it only where it's really important. For instance we have f1 and
> f2, they are declared const, and then only two lines later the constness has to be cast
> away again. I find that a bit over the top.

f1 and f2 can be declared immutable, because they don't need to change.

The later cast was a mistake of mine, I am sorry. In real-world D code in a situation like that I do as you have done, making f1 and f2 mutable.

But on Rosettacode I think it's important to underline the current problems in the D language itself. In an hypothetical future version of D if you cancat two immutable arrays you should get something that is typed as immutable, but is implicitly castable to mutable :-)

I will think a bit more about this. Maybe I will just make f1 and f2 mutable, it's the cleanest solution.


>> - "if (digits.length % 2)" instead of "if (digits.length & 1)": for the compiler they are exactly the same, because
>> the value is unsigned. And using a modulus is more clear here. We are discussing about parity, not about bits.
>
> That is a matter of opinion. If I see "% 2", my first thought is: we're checking for even... then I realize the
> "== 0" is missing.

I will add the == 0 too then.


> I wrote the task myself, so I should know.

Ah :-) I didn't know. OK. So it's the Python entry to be wrong and yours is correct.


> It's fine with me. I'm glad we got rid of the ugly cast.

I agree, that cast in D was a bug of mine. (I try to fix mistakes in your code, but once in a while my code introduces other bugs/problems. In such cases on GitHub you can annotate code, but on mediawiki you have to just "fix" the code. It wasn't designed for code.)

Later I will change the code on Rosettacode.

Bye,
bearophile

> In the vampire code this doesn't happen because you return null (that is represented by two zero words), but in general unlike Python in D the safe and idiomatic way to test for array emptiness is to use std.array.empty. Otherwise you risk having problems.

I have just opened a thread in the main D newsgroup about this:

http://forum.dlang.org/thread/bwgnbflygowctlisistg@forum.dlang.org

Bye,
bearophile

This task has just a Tango entry:
http://rosettacode.org/wiki/Concurrent_computing


So I am writing a Phobos version. This seems to work as requested:


import std.stdio, std.random, std.parallelism, core.thread, core.time;

void main() {
    foreach (m; ["Enjoy", "Rosetta", "Code"])
        task!((s) {
            Thread.sleep(uniform(0, 1000).dur!"msecs");
            s.writeln;
        })(m).executeInNewThread;
}



I have also tried with a parallel foreach, the syntax is cleaner, but to me it always print the strings in the given order (so it's not doing what the task requires), do you know why?


import std.stdio, std.random, std.parallelism, core.thread, core.time;

void main() {
    foreach (s; ["Enjoy", "Rosetta", "Code"].parallel(1)) {
        Thread.sleep(uniform(0, 1000).dur!"msecs"); // Can't use UFCS.
        s.writeln;
    }
}


Bye,
bearophile

On 27-3-2013 0:20, bearophile wrote:
> This task has just a Tango entry:
> http://rosettacode.org/wiki/Concurrent_computing
>
>
> So I am writing a Phobos version. This seems to work as requested:
>
>
> import std.stdio, std.random, std.parallelism, core.thread, core.time;
>
> void main() {
>      foreach (m; ["Enjoy", "Rosetta", "Code"])
>          task!((s) {
>              Thread.sleep(uniform(0, 1000).dur!"msecs");
>              s.writeln;
>          })(m).executeInNewThread;
> }
>
>
>
> I have also tried with a parallel foreach, the syntax is cleaner, but to me it always print the strings in the given order (so it's not doing what the task requires), do you know why?
>
>
> import std.stdio, std.random, std.parallelism, core.thread, core.time;
>
> void main() {
>      foreach (s; ["Enjoy", "Rosetta", "Code"].parallel(1)) {
>          Thread.sleep(uniform(0, 1000).dur!"msecs"); // Can't use UFCS.
>          s.writeln;
>      }
> }


Output on my system:


C:\test
Rosetta
Enjoy
Code

C:\test
Code
Enjoy
Rosetta

C:\test
Enjoy
Rosetta
Code

C:\test
Enjoy
Code
Rosetta

C:\test
Code
Enjoy
Rosetta

On 27-3-2013 15:17, Jos van Uden wrote:
> On 27-3-2013 0:20, bearophile wrote:
>> This task has just a Tango entry:
>> http://rosettacode.org/wiki/Concurrent_computing
>>
>>
>> So I am writing a Phobos version. This seems to work as requested:
>>
>>
>> import std.stdio, std.random, std.parallelism, core.thread, core.time;
>>
>> void main() {
>>      foreach (m; ["Enjoy", "Rosetta", "Code"])
>>          task!((s) {
>>              Thread.sleep(uniform(0, 1000).dur!"msecs");
>>              s.writeln;
>>          })(m).executeInNewThread;
>> }
>>
>>
>>
>> I have also tried with a parallel foreach, the syntax is cleaner, but to me it always print the strings in the given order (so it's not doing what the task requires), do you know why?
>>
>>
>> import std.stdio, std.random, std.parallelism, core.thread, core.time;
>>
>> void main() {
>>      foreach (s; ["Enjoy", "Rosetta", "Code"].parallel(1)) {
>>          Thread.sleep(uniform(0, 1000).dur!"msecs"); // Can't use UFCS.
>>          s.writeln;
>>      }
>> }
>
>
> (...) my system:

DMD32 D Compiler v2.062

win7 64 bits, i7 2600

Jos van Uden:

> Output on my system:
>
>
> C:\test
> Rosetta
> Enjoy
> Code
>
> C:\test
> Code
> Enjoy
> Rosetta

Thank you for your test, I will replace the Rosettacode one with the nicer version.

I don't know why the second doesn't work correctly on my system, while the first works.

Bye,
bearophile

I you want to take a look, I've seen that my translation of the Python entry was tagged as wrong:

http://rosettacode.org/wiki/Set_puzzle#Alternative_Version

Bye,
bearophile

On 5-4-2013 14:23, bearophile wrote:
> I you want to take a look, I've seen that my translation of the Python entry was tagged as wrong:
>
> http://rosettacode.org/wiki/Set_puzzle#Alternative_Version

Ledrug tagged it. The output says: striped open open. That shouldn't happen.