[issue20825] containment test for "ip_network in ip_network"

Aleksandr Balezin report at bugs.python.org
Tue May 23 07:45:08 EDT 2017


Aleksandr Balezin added the comment:

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

----------
nosy: +gescheit

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue20825>
_______________________________________


More information about the Python-bugs-list mailing list