Skip to main content
1 of 4
SylvainD
  • 29.8k
  • 1
  • 49
  • 93

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.")

To be continued

SylvainD
  • 29.8k
  • 1
  • 49
  • 93