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

Consider using string as key for bkDict type objects ? #1

Closed sgmihai closed 1 year ago

sgmihai commented 1 year ago

I was thinking, wouldn't it be simpler to have string as bkDict keys, instead of BencodeObj ? According to wikipedia: https://en.wikipedia.org/wiki/Bencode#Encoding_algorithm "All keys must be byte strings and must appear in lexicographical order." For accessing a dict value now I have to write: myDecodedBeObj.d[Bencode("key")] instead of just myDecodedBeObj.d["key"]

Also, maybe publish to nimble directory ? There's already a nim-bencode library there, but it does not work correctly for all cases. And this one has clearer code

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

I think I made bkDicts have BencodeObj keys in order to keep the library as flexible as possible (e.g. some exotic application might decide to use non-strings as bencode dictionary keys) (though I don't really remember; I wrote the library a long time ago).

How about if I added overloads that automatically convert the given string key to a bencode object, allowing you to do stuff like this?:

var b = Bencode({
  "interval": Bencode(1800),
  "complete": Bencode(20),
})
assert b.d["interval"] == Bencode(1800)
b.d["complete"] = Bencode(30)

Now that I'm looking at this library again, I'm thinking of changing BencodeObj to a regular value type instead of a ref object. What would you think about that?

sgmihai commented 1 year ago

I already modified the code locally to use strings instead of Bencode("string") for the dict keys. The overload you are talking about looks like the best option though. About the ref vs value type, I am not really knowledgeable in this field and can't really grasp all the low level implications it would have. All I know is that with ref, assigning the dict to another variable, it won't to deep copy for the full tree when dereferncing.