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 <[email protected]>
<http://bugs.python.org/issue20825>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
