Validate IP4 address

Question

Validate IP Address

I got this problem during an interview. And would like to get some code review. I also wrote several tests with the expected output, and they all passed as expected.

Validate an IP address (IPv4). An address is valid if and only if it is in the form "X.X.X.X", where each X is a number from 0 to 255.

For example, "12.34.5.6", "0.23.25.0", and "255.255.255.255" are valid IP addresses, while "12.34.56.oops", "1.2.3.4.5", and "123.235.153.425" are invalid IP addresses.

Examples:

"""    

ip = '192.168.0.1'
output: true

ip = '0.0.0.0'
output: true

ip = '123.24.59.99'
output: true

ip = '192.168.123.456'
output: false
"""

def validateIP(ip):
  #split them by '.' , and store them in an array
  #check the array if the length is 4 length
  arr = ip.split('.')
  if len(arr) != 4:
    return False
  #0 check for special edge cases when non-digit
  #1. check if they are digit, 
  #2. check if check the integer is between 0 and 255
  
  for part in arr:
    if len(part) > 1:
      if part[0] == '0':
        return False
    if not part.isdigit():
      return False
    digit = int(part)
    if digit < 0 or digit > 255:
      return False
  return True

#case#0

ip0="08.0.0.0" # False
test0= validateIP(ip0)
print(test0)

#case#1
ip1 = "192.168.0.1"
test1 = validateIP(ip1)
print(test1)

#case#2
ip2 = '0.0.0.0'
test2 = validateIP(ip2)
print(test2)

#case#3
ip3 = '123.24.59.99'
test3 = validateIP(ip3)
print(test3)

#case#4
ip4 = '192.168.123.456'
test4 = validateIP(ip4)
print(test4)

#case5
ip5 = "255.255.255.255"
test5 = validateIP(ip5)
print(test5)

Answer 1

def validateIP(ip):

I would expect a name starting is (a useful hint that it returns a Boolean rather than some more complex validation data structure) and explicitly mentioning IP v4 (since the current name is misleading). E.g. is_valid_IPv4_address.

---

  #split them by '.' , and store them in an array
  #check the array if the length is 4 length
  arr = ip.split('.')
  if len(arr) != 4:
    return False

The comments don't tell me anything which the code doesn't already. In general, good comments explain why, not what.

---

  #0 check for special edge cases when non-digit
  #1. check if they are digit, 
  #2. check if check the integer is between 0 and 255

  for part in arr:
    .. various conditions which return False
  return True

IMO it would be more Pythonic to use all: I would boil the whole function down to

    parts = ip.split('.')
    return len(parts) == 4 and all(is_valid_IPv4_address_part(part) for part in parts)

---

    if len(part) > 1:
      if part[0] == '0':
        return False

This isn't in the spec. It's a reasonable constraint, but you should check with the person who gave you the spec before writing the code, or at least put in a comment saying that you're making an assumption about the true intentions of the specifier.

---

    if not part.isdigit():
      return False

This is buggy. (Before testing I thought there was an issue which should be bounced back to the specifier. Upon testing, I found that some of my test cases caused validateIP to throw an exception).

What is the expected output for these test cases?

¹.¹.¹.¹
١.١.١.١
𝟣.𝟣.𝟣.𝟣
①.①.①.①

Answer 2

1. A doc string reads nicer then # blockcomments

   Consider making a doc string of that function, so you can do help(validate_ip) and it will print the doc string in the interpreter.

2. Adhere to PEP8

   Functions and variables should be snake_case ie def validate_ip(ip):

3. You could use the all keyword to check if each part is correct; this will return False for the first failure.

4. Make actual tests that ensure validity

   Instead of printing tests, make actual tests either with assert or the modules doctest or unittest.

5. There is a module that does this for you

   Python is often described as "batteries included", and here you could use the ipaddress module, which will validate an IP when you create the IPv4Adress object.

Reworked code

import doctest

def validate_ip(ip):
    """
    Checks if the ip address is valid
    args:
        ip (str): The IP address
    ret:
        A boolean: True for a a valid IP
    
    >>> validate_ip('08.0.0.0')
    False

    >>> validate_ip('192.169.0.1')
    True

    >>> validate_ip('0.0.0.0')
    True

    >>> validate_ip('192.168.123.456')    
    False

    >>> validate_ip('oooh.0.0.1')
    False
    """
    ranges = ip.split('.')
    return len(ranges) == 4 \
        and all(
                r.isdigit() and                 # Check for digits
                int(r) in range(0, 256) and     # Check in range of 0-255
                (r[0] != "0" or len(r) == 1)    # Check for leading zero's
                for r in ranges
            )

if __name__ == '__main__':
    doctest.testmod()

ipaddress module

from ipaddress import IPv4Address

def is_valid_ip(ip):
    try:
        IPv4Address(ip)
        return True
    except ValueError:
        return False