On Fri, 29 Oct 2010 20:02:10 +0900, Daniel Murphy <[email protected]> wrote:

Some more things:
1. isArray should be isDynamicArray

Why? I think following code should work.

    ubyte[4] arr = [1, 2, 3, 4];
    Base64.encode(arr);

2. Need to import std.array for put(a, b)

No. std.range imports std.array as public.

3. The 4 implementations of encode could be merged into 2 using static
ifs.  This will result in some very ugly code, but will cut down on
code duplication and greatly simplify the method signature.  Same
thing for decode.

I don't think so. We have constraint for type separating.

4. The decoder chunk range takes both ubyte[] and char[], while the
decode function only takes ubyte[].  They should both probably take
the same types.

This is sensitive problem. encode method performs Base64 encoding, but Encoder itself doesn't.
Encoder is a convenient Range wrapper, so I loose the type limit.

5. Chunk version of decoder does not call doDecoding in the constructor.

I noticed this. I removed doDecoding for actual code and forgot to add doDecoding.

6. Chunk versions of encoder/decoder do not define save correctly
(should make a copy of the buffered data to prevent aliasing)

Oops.

7. The requirement that decoder should take chunks with a size that is
a multiple of 4 must either be asserted or removed.  The encoder range
needs a warning that padding will be added to the end of each chunk.
The warnings need to be much bigger and appear earlier in the
documentation for each range.

7.5.  Considering that it says chunks must be a multiple of 4, it is
not necessary for doDecoding to read more than one chunk from the
input.  This should eliminate all the extra allocations in Decoder.

I don't understand this point.
ByChunk doesn't concern Encoder.

8. The encoder and decoder shortcut functions probably need to have a
copy of the template constraints.  This will give better error
messages.

You are right. I will add common constraint.

9. The line Appender!(char[])([]); gives me an error about converting
arrays of void[].  Changing it to Appender!(char[])(null); gets rid of
it.  Is this still present with the newest dmd?

10. The block of unittests at 1450 still refuses to compile for me.
Again, this might be because I'm using 2.048.  The errors are about
using to!ubyte, and passing map to encode.

I don't know the legacy dmd.


Masahiro
_______________________________________________
phobos mailing list
[email protected]
http://lists.puremagic.com/mailman/listinfo/phobos

Reply via email to