Construct: VarInt integer encoding

Question

This code is part of the Construct library. Docstrings explain what the code is supposed to do.

@singleton
class VarInt(Construct):
    r"""
    Varint encoded integer. Each 7 bits of the number are encoded in one byte in the stream, having leftmost bit not set when byte is terminal.

    Scheme defined at Google's site:
    https://developers.google.com/protocol-buffers/docs/encoding
    https://techoverflow.net/blog/2013/01/25/efficiently-encoding-variable-length-integers-in-cc/

    Example::

        >>> VarInt.build(16)
        b'\x10'
        >>> VarInt.parse(_)
        16
        >>> VarInt.build(2**100)
        b'\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80\x80\x04'
        >>> VarInt.parse(_)
        1267650600228229401496703205376
    """
    def _parse(self, stream, context, path):
        acc = []
        while True:
            b = byte2int(_read_stream(stream, 1))
            acc.append(b & 0b01111111)
            if not b & 0b10000000:
                break
        num = 0
        for b in reversed(acc):
            num = (num << 7) | b
        return num
    def _build(self, obj, stream, context, path):
        if obj < 0:
            raise ValueError("varint cannot build from negative number")
        while obj > 0b01111111:
            _write_stream(stream, 1, int2byte(0b10000000 | (obj & 0b01111111)))
            obj >>= 7
        _write_stream(stream, 1, int2byte(obj))

some of the code refers to: https://github.com/construct/construct/blob/master/construct/lib/py3compat.py

Answer 1

• A class with no state is a good indication that the class is not needed. Free functions will do just fine.

• context and path play no role neither in parsing nor in building. Consider not to pass them.

• I don't see a need for two passes in parse. Consider

  def _parse(stream):
      num = 0
      while True:
          b = byte2int(_read_stream(stream, 1))
          num << 7
          num |= b & 0b01111111
          if not b & 0b10000000:
              break
      return num
  

• Instead of spelling out both 0b01111111 and 0b10000000 I recommend to spell just one, and make second its negation. I may recommend using familiar 0x7f and 0x80 hex masks instead (but this is a matter of taste).