16
\$\begingroup\$

While trying to learn Python I stumbled upon a question which asks you to write a code that is able to take an input and verify it to see whether it meets the IPv6 Criteria.

I wrote the following code:

valid_characters = ['A', 'B', 'C', 'D', 'E', 'F', 'a', 'b', 'c', 'd', 'e', 'f', ':', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9']
address = input('Please enter an IP address: ')
is_valid = []

for i in range(len(address)):
    current = address[i]
    for j in range(len(valid_characters)):
        check = valid_characters[j]
        if check == current:
            a = 1
            is_valid.append(a)

address_list = address.split(":")
invalid_segment = False

for i in range(len(address_list)):
    current = address_list[i]
    if len(current) > 4:
        invalid_segment = True

if len(address) == len(is_valid) and len(address_list) == 8 and invalid_segment == False:
    print("It is a valid IPv6 address.")

elif invalid_segment:
    print("It is not a valid IPv6 address.")

else:
    print("It is not a valid IPv6 address.")

Although the code works and for the sample cases that were provided it is successfully able to confirm whether the entered IP address is IPv6 or not, I feel like my solution is not really elegant enough and that there should be a better way to do this.

I would really appreciate it if someone could point me to the right direction for how I should condense or improve my code.

Please note that I am new to Python, with little to no experience.

\$\endgroup\$
  • 5
    \$\begingroup\$ Your code seems to reject ::1 and to accept 1::3:::6::8 \$\endgroup\$ – Hagen von Eitzen Apr 23 '18 at 16:43
  • 3
    \$\begingroup\$ It is legal to write the last 32 bits of an IPv6 address in quad-dotted notation. \$\endgroup\$ – kasperd Apr 23 '18 at 19:12
  • 2
    \$\begingroup\$ While there are good answers that deal with your code as it is, it should be remarked that the best solution would depend on the things you are trying to learn. If it is string manipulation, this is fine. If it would strive for efficiency your solution could avoid all the strings by dealing with int(, base=16) of input parts, which probably would be more efficient. Ultimately, of course, in real life solution you should probably deal with Python ipv6 classes, as suggested in another answer. \$\endgroup\$ – Gnudiff Apr 24 '18 at 9:22
27
\$\begingroup\$
  1. You can change valid_characters to a string. 'ABCDEFabcdef:0123456789'.
  2. You can use for element in elements: rather than:

    for i in range(len(elements)):
        element = elements[i]
    
  3. You can use in rather than manually perform the check.

    if current in valid_characters:
        is_valid.append(1)
    
  4. You can use a list comprehension, rather than manually build a list.

    is_valid = [
        1
        for current in address
        if current in valid_characters
    ]
    
  5. Rather than manually performing the any check you can use a comprehension to do it for you:

    invalid_segment = any(len(current) > 4 for current in address_list)
    
  6. Rather than checking the length, you can do the same to is_valid:

    is_valid = all(current in valid_characters for current in address)
    
  7. You can invert invalid_segment to simplify the logic later.

  8. You now simplify your checks. You don't need your elif as well.
  9. I'd only use one type of string opener and closer. I personally use '.

This leads to:

valid_characters = 'ABCDEFabcdef:0123456789'
address = input('Please enter an IP address: ')

is_valid = all(current in valid_characters for current in address)
address_list = address.split(':')
valid_segment = all(len(current) <= 4 for current in address_list)

if is_valid and valid_segment and len(address_list) == 8:
    print('It is a valid IPv6 address.')
else:
    print('It is not a valid IPv6 address.')

You can also make this a function. And so could use:

VALID_CHARACTERS = 'ABCDEFabcdef:0123456789'


def valid_ip6(address):
    address_list = address.split(':')
    return (
        len(address_list) == 8
        and all(len(current) <= 4 for current in address_list)
        and all(current in VALID_CHARACTERS for current in address)
    )


if __name__ == '__main__':
    address = input('Please enter an IP address: ')

    if valid_ip6(address):
        print('It is a valid IPv6 address.')
    else:
        print('It is not a valid IPv6 address.')
\$\endgroup\$
  • 2
    \$\begingroup\$ Very nice answer. Nitpick: I would also introduce the name == 'main' guard so that the OP gets used to it from an early stage. \$\endgroup\$ – magu_ Apr 23 '18 at 16:49
31
\$\begingroup\$

Python has a philosophy of "Batteries Included". Don't reinvent what's in the standard library!

In particular, this code is much more easily written if we take advantage of IPv6Address:

import ipaddress

address = input('Please enter an IP address: ')

try:
    addr = ipaddress.IPv6Address(address)
except ipaddress.AddressValueError:
    print(address, 'is not a valid IPv6 address')
else:
    if addr.is_multicast:
        print(address, 'is an IPv6 multicast address')
    if addr.is_private:
        print(address, 'is an IPv6 private address')
    if addr.is_global:
        print(address, 'is an IPv6 global address')
    if addr.is_link_local:
        print(address, 'is an IPv6 link-local address')
    if addr.is_site_local:
        print(address, 'is an IPv6 site-local address')
    if addr.is_reserved:
        print(address, 'is an IPv6 reserved address')
    if addr.is_loopback:
        print(address, 'is an IPv6 loopback address')
    if addr.ipv4_mapped:
        print(address, 'is an IPv6 mapped IPv4 address')
    if addr.sixtofour:
        print(address, 'is an IPv6 RFC 3056 address')
    if addr.teredo:
        print(address, 'is an IPv6 RFC 4380 address')
\$\endgroup\$
  • \$\begingroup\$ Another nice thing about this is that it simplifies the IP address for you by changing it to shorthand notation. \$\endgroup\$ – mbomb007 Apr 23 '18 at 13:38
  • 1
    \$\begingroup\$ And it handles uncommon forms such as embedded IPv4 (e. g. 2001:db8:122:344::192.0.2.33 = 2001:db8:122:344::c000:221). \$\endgroup\$ – Roman Odaisky Apr 23 '18 at 15:23
  • 16
    \$\begingroup\$ This is cheating and kills the purpose of the exercise ;-) \$\endgroup\$ – t3chb0t Apr 23 '18 at 18:14
10
\$\begingroup\$

Various comments in no specific order.


The 2 last statements are identical, thus it is not relevant to check invalid_segment.

You could write:

if len(address) == len(is_valid) and len(address_list) == 8 and invalid_segment == False:
    print("It is a valid IPv6 address.")
else:
    print("It is not a valid IPv6 address.")

It would be clearer to write a function returning a boolean to check the address. It makes your code easier to understand and easier to re-use. Also, you could move the part handling the input/output behind an if __name__ == "__main__": guard.

You'd get:

valid_characters = ['A', 'B', 'C', 'D', 'E', 'F', 'a', 'b', 'c', 'd', 'e', 'f', ':', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9']

def ipv6_addr_is_valid(address):
    is_valid = []

    for i in range(len(address)):
        current = address[i]
        for j in range(len(valid_characters)):
            check = valid_characters[j]
            if check == current:
                a = 1
                is_valid.append(a)

    address_list = address.split(":")
    invalid_segment = False

    for i in range(len(address_list)):
        current = address_list[i]
        if len(current) > 4:
            invalid_segment = True

    return len(address) == len(is_valid) and len(address_list) == 8 and invalid_segment == False

if __name__ == "__main__":
    address = input('Please enter an IP address: ')
    if ipv6_addr_is_valid(address):
        print("It is a valid IPv6 address.")
    else:
        print("It is not a valid IPv6 address.")

Usually, when you use indices, range and len to iterate over an array in Python, you're doing it wrong. I highly recommend watching the talk "Loop Like A Native" from Ned Batchelder, it will help to write more concise, faster, easier to reuse code. In a nutshell, the for loop acts as a foreach loop so most of the time, you don't need to handle indices at all.

You'd get something like:

def ipv6_addr_is_valid(address):
    is_valid = []

    for current in address:
        for valid in valid_characters:
            if valid == current:
                a = 1
                is_valid.append(a)

    address_list = address.split(":")
    invalid_segment = False

    for current in address_list:
        if len(current) > 4:
            invalid_segment = True

    return len(address) == len(is_valid) and len(address_list) == 8 and invalid_segment == False

You use a is_valid list and use its number of elements to know whether the address is valid. Things could be done in a more efficient way. Let's do it in many small steps for education purposes.

  • you could count values instead of adding them to a list

    nb_valid = 0
    
    for current in address:
        for valid in valid_characters:
            if valid == current:
                nb_valid += 1
    
    ...    
    return len(address) == nb_valid and len(address_list) == 8 and invalid_segment == False
    
  • you could break out of the valid_characters loops as soon as you've found a match.

  • in Python, for loops accept a else part meaning "this loop exited the normal way: it didn't encounter a break". You could use this to return False as soon as a character is invalid.

    for current in address: for valid in valid_characters: if valid == current: nb_valid += 1 break else: # nobreak - invalid character return False

Thus, you don't even need to count nb_valid anymore:

for current in address:
    for valid in valid_characters:
        if valid == current:
            break
    else:  # nobreak - invalid character
        return False
  • You could use in to check if character is valid:

    for current in address: if current not in valid_characters: return False

  • You could initialise valid_characters in a more concise way in a single string:

    valid_characters = 'ABCDEFabcdef:0123456789'

then you could make the in lookup for efficient by using a set:

valid_characters = set('ABCDEFabcdef:0123456789')

You could break after finding an invalid segment. You could also return False directly and get rid of the variable:

for current in address_list:
    if len(current) > 4:
        return False  # Invalid segment

At this stage, the code looks like:

valid_characters = set('ABCDEFabcdef:0123456789')

def ipv6_addr_is_valid(address):
    for current in address:
        if current not in valid_characters:
            return False


    address_list = address.split(":")

    for current in address_list:
        if len(current) > 4:
            return False  # Invalid segment

    return len(address_list) == 8

if __name__ == "__main__":
    print(ipv6_addr_is_valid("0:0:0:0:0:0:0:1"))
    # address = input('Please enter an IP address: ')
    # if ipv6_addr_is_valid(address):
    #     print("It is a valid IPv6 address.")
    # else:
    #     print("It is not a valid IPv6 address.")

You could use all/any to make some parts of your code more concise. It doesn't help too much in your case because you are using return already.

def ipv6_addr_is_valid(address):
    if any(c not in valid_characters for c in address):
        return False
    address_list = address.split(":")

    if any(len(c) > 4 for c in address_list):
        return False  # Invalid segment

    return len(address_list) == 8

Which can be re-organised:

def ipv6_addr_is_valid(address):
    address_list = address.split(":")
    return len(address_list) == 8 and all(c in valid_characters for c in address) and all(len(c) <= 4 for c in address_list)
\$\endgroup\$
  • \$\begingroup\$ Why would you pick the worst way to use any and all? You can move adress_list above, and use one return if you use all instead. \$\endgroup\$ – Peilonrayz Apr 23 '18 at 10:08
  • \$\begingroup\$ @Peilonrayz I wanted to perform changes little by little and show different techniques as OP said he was new to Python. \$\endgroup\$ – SylvainD Apr 23 '18 at 11:20
8
\$\begingroup\$

According to the Wikipedia article on IPv6 addresses, there are alternative ways of representing IPv6 addresses that you are not taking into account with this approach:

Leading zeroes in a group may be omitted, but each group must retain at least one hexadecimal digit.

...

One or more consecutive groups containing zeros only may be replaced with a single empty group, using two consecutive colons (::).

...

The localhost (loopback) address, 0:0:0:0:0:0:0:1, and the IPv6 unspecified address, 0:0:0:0:0:0:0:0, are reduced to ::1 and ::, respectively.

\$\endgroup\$
6
\$\begingroup\$

Don't re-invent the wheel, use the netaddr module. This returns True / False based on the IP being valid or not:

import netaddr

def validate_v6_ip(ip_str):
    """Function for validating IPv6 addresses.

        validate_v6_ip(<IPv6 address string variable>)
    """
    assert type(ip_str) == str, 'Input must be a string.'
    return netaddr.valid_ipv6(ip_str.strip())
\$\endgroup\$
3
\$\begingroup\$

Properly validating an IPv6 address is tricky. Your code does not do it properly. It rejects valid cases and accepts invalid ones.

If I had to do it myself I think I would take the following approach.

  • Split the string on ':'
  • Check for a "dotted decimal quad" in the last part of the address. If one is found replace the last part with two hexadecimal parts.
  • Next check for abbriviation. Abbreviations can happen at the start (e.g. ::1), the end (e.g. 2001::), somewhere in between (e.g. 2001::1) or as the only thing in the address (::). You need to handle all these cases without allowing bogus abbreviations.
  • Validate that the number of parts in the address is acceptable. The range of acceptable part counts will depend on whether the address was abbreviated.
  • Finally validate the individual parts.

But as has been said unless this is an excercise it's probablly better to punt it to a library.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.