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. 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.
- 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.
- Adding immutable/const to the foreach loop variable "foreach
(n; [...])": D currently gives a warning if you try to mutate it.
In future D will allow you to mutate it, but it's only a copy, as
in Python. This is potentially confusion. Marking it as
const/immutable should become a standard D coding convention to
avoid troubles. It's not useless.
- "immutable q = k / i;" instead of "long q = k / i;" all
variables in a D program that don't need to mutate should be
annotated with const/immutable. This gives more optimization to
the compiler, helps avoid bugs of unwanted mutation later, and
makes code simpler to understand, because when you read code you
are sure something is not changing. It's explained here, among
other places:
http://blog.knatten.org/2011/11/11/disempower-every-variable/
- "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.
- "immutable f1 = getDigits(pair[0]);" instead of "auto f1 =
getDigits(pair[0]);": as before, f1 doesn't need to change, so it
should be immutable (I have used const, but immutable is better,
because getDigits is now pure).
- Annotations like "pairs ~= [i, q]; // Heap-allocated pair.":
they are useful because that little 2 items array is allocated on
the heap. It's good to remember us an important inefficiency in
the code. If you use a 2-tuple such inefficiency vanishes.
Someday hopefully this significant inefficiency of array literals
will be removed, and the comment will be removed.
- "// Unnecessary cast. Array concat allocates a new array." +
"if (!(cast(long[])(f1 ~ f2)).sort().equal(digits))" instead of "
if(!equal(digits, (f1 ~ f2).sort()))": f1 and f2 are immutable,
and currently if you concatenate them you get an immutable array,
that you can't sort. I agree the cast is bad here, so I have to
use dup instead. In future this problem will be hopefully
removed, so the code will be fixed and the comment removed.
RosettaCode is not just a show of D code, it's also a place to
help us improve D language itself. So hundreds of D entries in
Rosettacode have annotations like that, that help us remember
limits or bugs or problems in the D language. I have removed tens
of similar annotations when D bugs get fixed. They are useful for
the development od D.
(Here I have listed only the things that I think should be
reverted. There are other things in my version of the problem
that are more like personal style preferences that I have not
listed here, that are used in most or all the other D entries.)
Bye,
bearophile
