rust-bencode icon indicating copy to clipboard operation
rust-bencode copied to clipboard

Name conflict between 2 ByteString's

Open dtantsur opened this issue 11 years ago • 6 comments

There are 2 things in this library called ByteString: enum item ByteString and type util::ByteString used as a key. Placing in util:: mod looks like it's something auxiliary while actually it's necessary part of the core functionality (I can hardly imagine implementing To/FromBencode without using it explicitly). I am working around this like:

use bencode;
use bencode::util::ByteString;
...

Now I have ByteString and bencode::ByteString, both from bencode package. Guess which one is which :) Judging by use case, the could should be Key (why you didn't use it as initally wanted?) or smth like BytesHelper.

Could you rename one of ByteString's and move the second one to the top level of your package?

dtantsur avatar Oct 12 '14 13:10 dtantsur

Actually after thinking a bit more, I'd suggest renaming items in the enum (especially since you already had name conflict with Dict) like:

pub enum Bencode {
    EmptyValue,
    NumberValue(i64),
    ByteStringValue(Vec<u8>),
    ListValue(List),
    DictValue(Dict),
}

ByteString can stay the same (though reexported on top level). What do you think?

dtantsur avatar Oct 12 '14 17:10 dtantsur

I'll take a look at this tomorrow.

arjantop avatar Oct 16 '14 17:10 arjantop

How about renaming util::ByteString to Key or ByteStringKey?

arjantop avatar Oct 31 '14 21:10 arjantop

Key looks good. Btw with today's approved rfc about enum namespacing you'll soon be able to get rid of Value ending in enum elements.

dtantsur avatar Oct 31 '14 22:10 dtantsur

I know this is an old issue, but why do we even need util::ByteString? Can't we just Vec<u8> directly?

canndrew avatar Apr 17 '15 02:04 canndrew

@canndrew The problem is that it is not easy to implement this it with the current Encodable/Decodable api. What would have to be done is to just make a mark when the method emit_seq is called and then have a check in every other method to see if the sequence value is u8 (encode as bytestring) or encode it as a list of values.

arjantop avatar Apr 20 '15 15:04 arjantop