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.
