z------------- / nim-bencode

Bencode encode/decode library with serialization to/from JSON. Also includes a command-line program.
MIT License
2 stars 0 forks source link

Remove quotes from stringify string holding BencodeObj function #4

Closed sgmihai closed 1 year ago

sgmihai commented 1 year ago

https://github.com/z-------------/nim-bencode/blob/9a076dcf39d4c1c106096c6ca09d5b66684a3ca4/src/bencodepkg/types.nim#L45 You append quotes here to get the string representation, ([] and {} for seq, dict, which kind of makes sense visually) but given that you do not actually use strings but bencodeobj as keys in the table, we might need/use this stringified value as actual literal key in the code. When iterating over keys in a table, we get a BencodeObj, with quotes added to its string representation ($). I guess I could use key.s to get the literal string but this is another side effect of using BencodeObj as key. I know you said you wanted it "versatile", but that's not bencode, and it causes some unpleasentness, like not being able to construct a bencoded object with keys in alphabetical order by simply using .sort() on the ordered table, because there's no < and == operations defined for BencodeObj, so you have to define those (for bkString type). A self sorting structure, on modification, would be a cool thing to have by the way, so you are guaranteed to have a bencode conform result (even your example in the Readme is not sorted btw). So I'd suggest you remove the quotes from .toString func for strings, and maybe give another thought about using BencodeObj for keys instead of strings like it's supposed to be. Probably 1 in 10.000 people will want to have a bencode obj as key instead of the string, for whatever reason, and given that this library has maybe 2 users, that will never happen :). And it will make life easier for those two using it.

Also, the example in the readme needs import tables to work btw.

z------------- commented 1 year ago