Return to Answer

##Profiling comes before optimisation

Before making your program more efficient you have to make sure where those inefficiencies come from.

For example, it might be that your system is has to check lots of entries that were aggregated through several stores - so lots of valid GTINs that must be rejected because they are not in your list. Then, optimizing these lookups would be a top priority.

It might be that your system is feeded with lots of GTIN-looking things, so checking for validity is a top priority.

##Lookups

I assume that 'inefficiency' remark from your teacher referred to reading the file in every iteration of the loop. This is inefficient indeed. File should be read outside of the loop, once.

However, having a list and checking if an element is there is not really efficient too. There are other data structures, better suited for that. I'd store those GTINs in a set so that lookups are fast.

##Readability It is important. Sure, some optimizations make code less readable, but usually they are not worth it.

##Logic It seems that invalid entries sometimes (e.g. non-numeric ones) trigger no response at all and sometimes (checksum failed) trigger 'Product Not Found' response. Is that expected behaviour?

Compiling that together I'd get something like this:

def is_valid_GTIN(string):
 if string.is_numeric() and 8 == len(string):
 GTIN = [int(digit) for digit in string]
 if (3 * sum(GTIN[0::2]) + sum(GTIN[1::2])) % 10 == 0:
 return True
 return False
def get_input():
 pass
with open('read_it.txt') as input_file:
 known_GTINs = set(line.strip() for line in input_file)
 while True:
 GTIN = get_input()
 if is_valid_GTIN(GTIN):
 if GTIN in known_GTINs:
 print(GTIN)
 else:
 print('Product not found')
 else:
 print('Invalid GTIN')

With whatever fancy user-interaction I need to get input from user in place of get_input

Profiling comes before optimisation

Before making your program more efficient you have to make sure where those inefficiencies come from.

For example, it might be that your system is has to check lots of entries that were aggregated through several stores - so lots of valid GTINs that must be rejected because they are not in your list. Then, optimizing these lookups would be a top priority.

It might be that your system is feeded with lots of GTIN-looking things, so checking for validity is a top priority.

Lookups

I assume that 'inefficiency' remark from your teacher referred to reading the file in every iteration of the loop. This is inefficient indeed. File should be read outside of the loop, once.

However, having a list and checking if an element is there is not really efficient too. There are other data structures, better suited for that. I'd store those GTINs in a set so that lookups are fast.

Readability

It is important. Sure, some optimizations make code less readable, but usually they are not worth it.

Logic

It seems that invalid entries sometimes (e.g. non-numeric ones) trigger no response at all and sometimes (checksum failed) trigger 'Product Not Found' response. Is that expected behaviour?

Compiling that together I'd get something like this:

def is_valid_GTIN(string):
 if string.is_numeric() and 8 == len(string):
 GTIN = [int(digit) for digit in string]
 if (3 * sum(GTIN[0::2]) + sum(GTIN[1::2])) % 10 == 0:
 return True
 return False
def get_input():
 pass
with open('read_it.txt') as input_file:
 known_GTINs = set(line.strip() for line in input_file)
 while True:
 GTIN = get_input()
 if is_valid_GTIN(GTIN):
 if GTIN in known_GTINs:
 print(GTIN)
 else:
 print('Product not found')
 else:
 print('Invalid GTIN')

With whatever fancy user-interaction I need to get input from user in place of get_input