Message294244
I've reviewed this patch and want to make some advices.
- hasattr is unwanted here. There is no any similar usage hasattr in this module. Also before hasattr there is a call of _ipversion method. If other is not instance of BaseNetwork it will raise AttributeError exception before hasattr check.
- It is not a good manner comparing thru "other.network_address >= self.network_address and other.broadcast_address <= self.broadcast_address"(see issue25430). Networks must be compared thru "other._prefixlen >= self._prefixlen and other.network._ip & self.netmask._ip == self.network._ip" for performance reason.
- _containment_check function is excessive. There is no common logic in supernet_of/subnet_of function except _ipversion and type checking. I think this two functions should be simple as:
def subnet_of(self, other):
if self._version != other._version:
raise TypeError('%s and %s are not of the same version' % (self, other))
if other._prefixlen >= self._prefixlen and other.network._ip & self.netmask._ip == self.network._ip:
return True
return False |
|
Date |
User |
Action |
Args |
2017-05-23 11:45:08 | gescheit | set | recipients:
+ gescheit, ncoghlan, pitrou, eric.smith, pmoody, r.david.murray, SilentGhost, berker.peksag, exhuma, JamesGuthrie, James Schneider |
2017-05-23 11:45:08 | gescheit | set | messageid: <1495539908.42.0.53744975869.issue20825@psf.upfronthosting.co.za> |
2017-05-23 11:45:08 | gescheit | link | issue20825 messages |
2017-05-23 11:45:08 | gescheit | create | |
|