Change decoding functions

[synlestidae]

Two key changes here

• Each decoding function now borrows the bytes that it decodes, rather than consuming them
• Each now uses a mutable position parameter. This is incremented by the number of bytes that are parsed.

What do you think? I thought these changes might make implementing the bdict_decode function easier, but I got a little carried away

UPDATE

I implemented the bdict_decode function. It's not tested very well - only has one test. Ready for encoding now!

[MatejLach]

Does position need to be taken by reference?
From what I can see you're using it to move around global state of the position.
I don't think that's idiomatic Rust.
Primitive types like usize implement shallow Copy and are allocated on the stack, so it should be possible to just have straight by-value mut usize and then return the current position as part of a tuple, without loosing efficiency.

[MatejLach]

You're fast @synlestidae, awesome work! 👍
Great changes and I agree that making the decoding functions borrow their bytes is much more idiomatic.
I've made a number of comments, they're mostly suggestions, I am open to different ideas.
In particular however, I am not sure about position being a reference.

Let me know what you think.
Thanks!

[synlestidae]

Quite keen on this project so have been working enthusiastically on it haha. I've read your comments, will take them on board. Regarding position as a reference, I did it because it simplifies accounting for the extra element in the return type. But you've convinced me that the return type should have the new position now.

[MatejLach]

@synlestidae Haha, nice!
When you're all done, could you please squash some of the commits, (i.e some of the error handling stuff), into one and just let me know when to merge, thanks.

Regarding the position pointer, I don't feel super strongly about it (so if you are not 100% convinced, keep the reference),, but I just think that in a good API, it should be clear entirely from the function signature itself, what it does. The position reference has a bit of a magic feel to it.
Something like position_arg: mut usize) -> Result<(BString, usize), DecodeError> makes it explicit and easy to digest just from the function signature what's going on.

I hope to find time as well, so that if we can do encoding this week, perhaps we can start on the Peer-Wire protocol soon, which is the really interesting part.

[synlestidae]

Yes I didn't have time to change the position pointer (which I want to do) but will do so later on today after work. And I'll squash the commits too.

[MatejLach]

Thanks! 👍