<p>11. Please give me a diff file.</p>
<p>I cannot use a Mac, so I will reply other things later.<br></p>
<p>Masahiro </p>
<p><blockquote type="cite">2010/10/29 20:03 "Daniel Murphy" <<a href="mailto:yebblies@gmail.com">yebblies@gmail.com</a>>:<br><br><p><font color="#500050">On Fri, Oct 29, 2010 at 6:27 PM, Masahiro Nakagawa <<a href="mailto:repeatedly@gmail.com">repeatedly@gmail.com</a>> wrote:<br>
><br>> You are right. ...</font></p>Great!<br>
I think it should be "'\\0'..." rather than "'\0'...", otherwise it<br>
will just embed a null character into the error.<br>
<br>
There should also be another space:<br>
static assert(Padding < '0' || Padding > '9', "Character '" ~ Padding<br>
~ "'cannot be used twice");<br>
static assert(Padding < '0' || Padding > '9', "Character '" ~ Padding<br>
~ "' cannot be used twice");<br>
<p><font color="#500050"><br><br>><br>> Your suggestion increases struct size and decreases performance.<br>> But, I agree.<br>><br>> <a href="http://bi.">http://bi.</a>..</font></p>It's not very pretty, but without it encoder and decoder don't<br>
correctly implement the range interface.<br>
<br>
Some more things:<br>
1. isArray should be isDynamicArray<br>
2. Need to import std.array for put(a, b)<br>
<br>
3. The 4 implementations of encode could be merged into 2 using static<br>
ifs. This will result in some very ugly code, but will cut down on<br>
code duplication and greatly simplify the method signature. Same<br>
thing for decode.<br>
<br>
4. The decoder chunk range takes both ubyte[] and char[], while the<br>
decode function only takes ubyte[]. They should both probably take<br>
the same types.<br>
<br>
5. Chunk version of decoder does not call doDecoding in the constructor.<br>
<br>
6. Chunk versions of encoder/decoder do not define save correctly<br>
(should make a copy of the buffered data to prevent aliasing)<br>
<br>
7. The requirement that decoder should take chunks with a size that is<br>
a multiple of 4 must either be asserted or removed. The encoder range<br>
needs a warning that padding will be added to the end of each chunk.<br>
The warnings need to be much bigger and appear earlier in the<br>
documentation for each range.<br>
<br>
7.5. Considering that it says chunks must be a multiple of 4, it is<br>
not necessary for doDecoding to read more than one chunk from the<br>
input. This should eliminate all the extra allocations in Decoder.<br>
<br>
8. The encoder and decoder shortcut functions probably need to have a<br>
copy of the template constraints. This will give better error<br>
messages.<br>
<br>
9. The line Appender!(char[])([]); gives me an error about converting<br>
arrays of void[]. Changing it to Appender!(char[])(null); gets rid of<br>
it. Is this still present with the newest dmd?<br>
<br>
10. The block of unittests at 1450 still refuses to compile for me.<br>
Again, this might be because I'm using 2.048. The errors are about<br>
using to!ubyte, and passing map to encode.<br>
<br>
11. Some of the documentation comments are not very good english<br>
(sorry). I assume you are not a native english speaker, I can update<br>
them if you would like.<br>
<br>
Thanks!<br>
<p><font color="#500050">_______________________________________________<br>phobos mailing list<br><a href="mailto:phobos@puremagic.com">phobos@puremagic.com</a><br><a href="http://list.">http://list.</a>..</font></p></blockquote>
</p>