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\$