In order to be trully versatile, we should be able to parametrize the building of this bitfield by its length. Unfortunately, integers are immutable and we can modify them in their __init__ method integers are immutable and we can modify them in their __init__ method. So we need to use __new__ instead:
In order to be trully versatile, we should be able to parametrize the building of this bitfield by its length. Unfortunately, integers are immutable and we can modify them in their __init__ method. So we need to use __new__ instead:
In order to be trully versatile, we should be able to parametrize the building of this bitfield by its length. Unfortunately, integers are immutable and we can modify them in their __init__ method. So we need to use __new__ instead:
#Interface usage
Given an AppliancePerms instance app_perm, you have app_perm.rest, app_perm.ssh and app_perm.cmd accept and return integers while app_perm.permissions accept an integer but return a string. This is, at best, confusing.
Furthermore, reading at the variable names, the intended usage for setting rest, ssh and cmd is to use app_perm.ssh = True or app_perm.cmd = False whereas getting the value app_perm.ssh will return 2 or 0.
You should improve the consistency here:
- return booleans when accessing individual fields;
- return integers when accessing the whole permissions.
#Proper formatting
On the other hand, I understand that you would like to have an pretty rendering of the selected bits. One way of doing it, is returning a subclass of int that will show itself as a bitfield:
class BitField(int):
def __repr__(self):
return '{:0{}b}'.format(self, 3)
You’ll note that I simplified the formating from integer to string. str.format is very versatile and you should favor it instead of manually manipulating string. Usage of this class being:
>>> five = BitField(5)
>>> print(five)
5
>>> str(five)
'5'
>>> repr(five)
'101'
>>> five
101
In order to be trully versatile, we should be able to parametrize the building of this bitfield by its length. Unfortunately, integers are immutable and we can modify them in their __init__ method. So we need to use __new__ instead:
class BitField(int):
def __new__(cls, value, length):
the_integer = super().__new__(cls, value)
the_integer._field_length = length
return the_integer
def __repr__(self):
return '{:0{}b}'.format(self, self._field_length)
#Parsing provided permissions
In the same vein, using app_perms.permissions = int('101', 2) is unfriendly. So you should parse it in your @permissions.setter. Getting a bitfield and setting it back should obviously work, but you could support being assigned a string or even an integer reprensenting a bitfield so '101', 101 or BitField(5) could work the same.
#Proposed improvements
class BitField(int):
def __new__(cls, value, length):
the_integer = super().__new__(cls, value)
the_integer._field_length = length
return the_integer
def __repr__(self):
return '{:0{}b}'.format(self, self._field_length)
class AppliancePerms(object):
"""Permissions for the Appliance class."""
_cmd_right = 2 ** 0
_rest_right = 2 ** 1
_ssh_right = 2 ** 2
_num_of_rights = 3
_all_rights = _cmd_right + _rest_right + _ssh_right
def __init__(self, perm_digits=None):
# default to all rights
if perm_digits is None:
self._perms = AppliancePerms._all_rights
else:
self._perms = perm_digits
@property
def permissions(self):
return BitField(self._perms, AppliancePerms._num_of_rights)
@permissions.setter
def permissions(self, perm_digits):
self._perms = int(repr(perm_digits), 2)
@permissions.deleter
def permissions(self):
self._perms = 0
@property
def rest(self):
return bool(self._perms & AppliancePerms._rest_right)
@rest.setter
def rest(self, boolean):
if boolean:
self._perms |= AppliancePerms._rest_right
else:
del self.rest
@rest.deleter
def rest(self):
if self._perms & AppliancePerms._rest_right:
self._perms -= AppliancePerms._rest_right
@property
def ssh(self):
return bool(self._perms & AppliancePerms._ssh_right)
@ssh.setter
def ssh(self, boolean):
if boolean:
self._perms |= AppliancePerms._ssh_right
else:
del self.ssh
@ssh.deleter
def ssh(self):
if self._perms & AppliancePerms._ssh_right:
self.perms -= AppliancePerms._ssh_right
@property
def cmd(self):
return bool(self._perms & AppliancePerms._cmd_right)
@cmd.setter
def cmd(self, boolean):
if boolean:
self._perms |= AppliancePerms._cmd_right
else:
del self.cmd
@cmd.deleter
def cmd(self):
if self._perms & AppliancePerms._cmd_right:
self._perms -= AppliancePerms._cmd_right