On 1/6/2011 12:07 PM, Michael Wood wrote:
Hi
On 6 January 2011 07:33, Tim Daly<d...@axiom-developer.org> wrote:
[...]
Take a look at http://daly.axiom-developer.org/clojure.pdf
I like it :)
Some simple corrections:
You have a typo on the front page:
"Based on Version 1.3.0-alphs4" (alphs4 instead of alpha4).
In the foreword, "This is a literate fork of Clojure, invented by Rich
Hickey." sounds a little like Rich Hickey invented this fork of
Clojure rather than Clojure itself.
You're right. Rich did not invent the fork. The spoon, perhaps.....
This is why projects need an Editor-in-Chief.
I have no idea what that "((iHy))" means after the date at the end of
the foreword.
It is an lisp joke between myself and certain others. Ignore it.
Page 2:
Last sentence of 1.3.2 says "We will look now look...".
In 1.3.3 the methods for updating the right member are identical to
the ones for the left member. I suspect "left" should be replace with
"right" in all three lines?
Page 5:
Fourth paragraph of 1.3.5 contains a sentence with a missing "a":
"Since it is copy operation...".
Also in 1.3.5, the list of Black node types duplicates the information
in 1.3.3. (Same with the list of Red nodes in 1.3.11.)
In the introductions to the Black, BlackVal, BlackBranch and
BlackBranchVal descriptions you say "This is constructed by
PersistenTreeMap's black method." Are all of them constructed with
the black method? If so, perhaps you should say that somewhere
instead of saying it separately for each one? Perhaps 1.3.10 is
enough. Just when I was reading through the introductions to Black,
BlackVal etc., I kept wondering if the next one was going to say it
was constructed by some method other than black, since you were
explicitly mentioning it every time.
Page 9:
There seems to be a spurious "new" in the first sentence: "If either
of the children exist but new have no value for this node..."
The second sentence of 1.3.11 is confusing to me. It seems to me that
the colour of the node returned has nothing to do with whether or not
subtrees are balanced, so it doesn't make sense to me to contrast
these two things. (Of course I know nothing about Red Black trees
other than what I've read so far in this document.)
The next paragraph is a little confusing to me too. Why would Red
nodes have to be returned only because the default colour is Black?
Surely even if the default colour were Red, Red nodes would need to be
returned? Perhaps I'm missing something fundamental about how Red
Black trees work.
One of the struggles about these paragraphs is that I am trying
to reverse engineer Rich's code. There are no comments to guide the
connection between ideas and code. I have to stare at it and guess
his intentions and mindset. Ideally code would be "born literate"
but that is not the case here.
The posted version
Page 13:
1.3.16 seems to be more or less a copy of 1.3.10 with Black replaced
with Red, so it also has the extraneous "new".
I did a self-study of errors once and found that 50 percent of all
the errors I ever make are caused by copy-pasting. You think I would
learn but apparently I don't.
Page 14:
The second sentence of 1.3.17 says "... all four cases all
balance...". It would be better without the second "all".
The next paragraph says "Each cases shows the ML code...". This
should of course be "Each case...". Also, what is "ML code"?
You're right. I should define all my abbreviations like ML before
they are used. I added a line to define it along with two literature
references.
Page 16:
There's a missing "'s" in the second sentence: "... a black node (p5)
removeRight method..."
Anyway, that's as far as I've got so far.
Very useful feedback. Thanks.
Tim Daly
--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clojure@googlegroups.com
Note that posts from new members are moderated - please be patient with your
first post.
To unsubscribe from this group, send email to
clojure+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
