lvh
commented
10 years ago I agree the not-super-obvious Python behavior should be explained, but I disagree that we can skip {>>, <<, &, ^, %}; not everyone has done bitwise or modular arithmetic :)
Open hmmueller opened 10 years ago
lvh
commented
10 years ago I agree the not-super-obvious Python behavior should be explained, but I disagree that we can skip {>>, <<, &, ^, %}; not everyone has done bitwise or modular arithmetic :)
hmmueller
commented
10 years ago Fine, that's why I wrote "that appendix could then contain a little more than were necessary for me, e.g., the bit operators or the % wrapping thing."
But the following (line 4732 in Crypto101.org - I hope I have the right version of Crypto101.org before me ...) is not an "explanation" of % - if you do not know what % is, you do not know why the following statement "wraps around" (whatever that then means, if you don't understand %).
The ~%~ in an expression like ~s[(i + n) % 624]~ means that a next
element of the state is looked at, wrapping around to the start of the
state array if there is no next element.(Moreover, the (i+n) should be (i+1), and "a next" should be "the next").
So I suggest to explain % "in a classic way," i.e., as a modulo operator on numbers; and then show how this is used to guarantee that an index expression points into an array ... and, to hit on the same nail again ;), do this in an appendix.
hmmueller
commented
10 years ago Re the explanation of the for/else: Sometimes, I love English for the way it uses prepositions.
when the ... loop isn't broken out of by the break statement
(Ok, "out" is not a preposition - but it ties in nicely with this artillery of single-syllable words connected to some grammatically far away part of the sentence ;) ...). I would prefer:
when the for loop isn't terminated by the break statement
lvh
commented
10 years ago Yeah, that is much better. I'm using that instead :)
Do you think it would make sense to just remove the comments from the code and have the Python appendix explain everything? (While I generally don't like appendices, I'd probably have to duplicate a lot if I don't. Plus: Python is a fairly popular language: there's probably plenty of people who understand things like the yield as-is :))
Most of the Python code is readable to anyone who know a C-like programming language. At a few places, however, Python constructs or idioms are used that are totally unknown to the non-Pythonian - I would suggest to either remove them or comment them.
Specifically, I found (up to now):
1) In the HKDF expansion code's get_output code, there is a for-else: construct. Consulting the web, it seems that even many Python coders do not understand this ("else is executed if loop not left by break" seems to be the correct semantics). As this is only for error handling here, which is of no real interest when explaining algorithms, I think that else:/raise part should be simply dropped.
2) In the Mersenne Twister's initialize_state method, state[-1] is used to access the last element of a list. Adding a comment would be very helpful
At the same place, >>, <<, &, and ^ are explained. But all these have been with us since the invention of C (i.e., when even I was only 5 or 6 years old!), and so everyone must know what that means (or look it up). So I would completely remove that "who haven't worked with Pyhton" paragraph. In the same vein, the explanation after "regenerate" that % helps to wrap around should be standard knowledge among programmers.
Of course, one could argue that it is necessary to explain that; but then, much more intricate/advanced concepts like
Maybe it would be a good idea to have an appendix: "The lesser-known features of Python I use in code snippets" or the like - then you could just point there; and that appendix could then contain a little more than were necessary for me, e.g., the bit operators or the % wrapping thing.