Part2 solutions (code only)

[chopmo]

No description provided.

[practicingruby]

Hi @chopmo,

I think that we can discuss more if you'd like once I post my solution, but generally this looks great! A couple minor concerns:

1. You used fixed size fields when packing things into messagepack format, but you don't have any validations. Since you know the max size of a fixstr, fixmap, etc., it'd probably make sense to at least raise a NotImplementedError for data that won't fit in that space. This kind of validation will guard against unexpected bugs, and serve as a useful reminder if/when you decided to expand the functionality to cover a more complete set of MessagePack formats.

2. You use pack("C*") for packing fixstr types into Ruby Strings. This will result in strings with binary encoding (i.e. no encoding), but the MessagePack spec calls for UTF-8. We're discussing on A minimal implementation of part 2 : msgpack #6 how to fix that.

Great work all around, hope those comments help.

[chopmo]

Thanks for your comments @sandal!

ad 1) Yes, leaving out the validations was too sloppy. These checks are obviously candidates for further unit testing, but I simply added the checks for now.
ad 2) I really like encoding stuff...when compared to timezone stuff ;) Added a force_encoding call which should fix this problem if I understand your discussion with @eregon correctly.

[eregon]

The unpack check is definitely good, but I think the pack one might be improved.
As we have built-in transcoding with String#encode, the simplest would be to utf8 = str.encode("UTF-8") and add utf8.bytes to the stream.
If I interpret correctly, your code checks if the bytes of the String are valid UTF-8 bytes and adds the original bytes to the stream. But the original bytes could be valid UTF-8 bytes, yet their interpretation quite different (the check of valid UTF-8 bytes is not strong enough to guarantee the string is encoded in UTF-8).

[eregon]

For instance:

> "駧".encode(Encoding::ISO8859_1).force_encoding(Encoding::UTF_8).valid_encoding?
=> true
> "駧".encode(Encoding::ISO8859_1).force_encoding(Encoding::UTF_8)
=> "駧"