Dec-27-2016, 11:34 PM
On top of wavic's comment, this requires internet access, right? The naming here, to me at least, implies that 8.8.8.8 should be considered valid with or without internet access. Exceptions seem like a weird methodology here, so false negatives seem possible in other situations as well (I haven't dug through the docs to see what exactly). It also means that with a shaky internet connection or something, you'll get inconsistent return values if you call this frequently, which would be surprising to a caller expecting idempotence. Lastly, in a loop, this will be much slower than non-IO means of similar checking. The scope in which this is meant to be used should definitely be documented at least. For some precedent if you think this isn't a big deal: http://brian.pontarelli.com/2006/12/05/m...uals-suck/
Code like
Both functions validipv* have similar bodies and can be refactored for DRYer code. You can save a line by returning right away too.
The 4+6 thing is bizarre; why not just use flags?
The return codes are bizarre as well, and not documented in the script. Since positive values indicate the count of failed IPs, and that *could* collide with the 98 value, you can use negative values one way or the other.
How can you hit the branch on line 97? You catch the IOErrors at lower levels, right? Similarly, for line 102, the return value is always an int, so that should not happen either, right?
You have an unused import, I recommend you use a linter.
Code like
def validip(addr): if validipv4(addr): return True if validipv6(addr): return True return Falsecan be written more succinctly
def validip(addr): return validipv4(addr) or validipv6(addr)(generally, logic around returning Booleans can be simplified like this)
Both functions validipv* have similar bodies and can be refactored for DRYer code. You can save a line by returning right away too.
The 4+6 thing is bizarre; why not just use flags?
The return codes are bizarre as well, and not documented in the script. Since positive values indicate the count of failed IPs, and that *could* collide with the 98 value, you can use negative values one way or the other.
How can you hit the branch on line 97? You catch the IOErrors at lower levels, right? Similarly, for line 102, the return value is always an int, so that should not happen either, right?
You have an unused import, I recommend you use a linter.