Skip to main content
Code Review

Return to Revisions

4 of 4
replaced http://stackoverflow.com/ with https://stackoverflow.com/

I guess this just for exercise, since there are already quite a number of ways to serialise Python data. And since cPickle is written in C it's unlikely that comparable performance will be achieved by pure (c)Python code.

Since this is Python code, the StoreData class should probably be hidden inside a function, unless you want to mock it for some reason. I.e. if there's no need for a class (and there's none because it's stateless), don't use one.

Also, since it claims to handle list objects, I'd consider the following behaviour a bug:

x = [42]
x[0] = x
StoreData().save(x)

Serialisers usually need to track object identities because of that.


Now for the code as presented:

  • round_up is math.ceil.
  • int_to_bin should probably use this SO answer instead.

There should probably also be a very long lists of tests to make sure that the serialisation works as expected and e.g. on both Python 2 and 3 for that matter.

In general overly long methods like _encode_value should be split into meaningful parts. As a reader it's very hard to keep track of all the different things that happen in both of them.

I'd suggest to splitting out size calculation, headers and any common code into methods.

Also, since this is Python 2, consider xrange and any other generators instead of allocating lists.


For possible extensions, consider the changes you'd need to make to support serialisation of custom classes / objects.

ferada
  • 11.4k
  • 25
  • 65
default

AltStyle によって変換されたページ (->オリジナル) /