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)
::1and to accept1::3:::6::8\$\endgroup\$