Author gescheit
Recipients James Schneider, JamesGuthrie, SilentGhost, berker.peksag, eric.smith, exhuma, gescheit, ncoghlan, pitrou, pmoody, r.david.murray
Date 2017-05-23.11:45:08
SpamBayes Score -1.0
Marked as misclassified Yes
Message-id <1495539908.42.0.53744975869.issue20825@psf.upfronthosting.co.za>
In-reply-to
Content
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
History
Date User Action Args
2017-05-23 11:45:08gescheitsetrecipients: + gescheit, ncoghlan, pitrou, eric.smith, pmoody, r.david.murray, SilentGhost, berker.peksag, exhuma, JamesGuthrie, James Schneider
2017-05-23 11:45:08gescheitsetmessageid: <1495539908.42.0.53744975869.issue20825@psf.upfronthosting.co.za>
2017-05-23 11:45:08gescheitlinkissue20825 messages
2017-05-23 11:45:08gescheitcreate