[Tutor] Re: Simpler way to do this (for-loop)?

[Andrei]

Olli Rajala <olli.s.rajala <at> tut.fi> writes:

> <snip>how to refactor<snip>:
> if not category:
> 	print '<option selected>(Choose the category)</option>'
> 	for line in categories:
> 		print '<option>%s</option>\n' % (line)
> else:
> 	print '<option>(Choose the category)</option>'
> 	for line in categories:
> 		if line == category:
> 			print '<option selected>%s</option>\n' % (line)
> 		else:
> 			print '<option>%s</option>\n' % (line)

When refactoring, always look for duplicate code in different if-statements
and such. In this case, you see that the "Choose the category" thing
appears in both parts of the if statement and also the printing of
non-selected options.

Simply remove the outer if-condition - it serves no purpose. The else part
of the condition already automatically covers the case of an unspecified
category; it will not display anything as selected, given the fact
that line will never match an empty category. 
You could also move the format string into a separate variable and get 
the selected status from a dictionary, though I'm not sure that is an
improvement over using the if-statement inside the loop.

print '<option>(Choose the category)</option>'
s = '<option%s>%s</option>\n'
d = {True: ' selected', False: ''}
for line in categories:
    print s % (d[line==category], line)

Yours,

Andrei