patternpythonMinor
Optimize iteration output to text file
Viewed 0 times
filetextoutputiterationoptimize
Problem
I've written a piece of code which combines given letters in groups of n letters, and writes all the combinations in a text file. This is my first attempt and I wonder if there is an optimal way to do this.
letter_input = raw_input(' Type in letters: ')
number_input = int(raw_input(' Letters per word: '))
split_letters = list(letter_input)
def function():
all_combinations_list = list(itertools.permutations(split_letters, number_input))
output_file = open('/tmp/list.txt', 'w')
number_of_combinations = len(all_combinations_list)
for n in range(0,number_of_combinations):
raw_characters = str(all_combinations_list[n])
out_1 = raw_characters.translate(string.maketrans('',''), string.punctuation)
out_2 = out_1.replace(' ','')
print >>output_file, out_2
output_file.close()Solution
Don't use global variable
It make things hard to understand and to maintain. In your case, the simple way to do this is to feed your function
Make your code easier to read
You can ensure you respect PEP 8, the guideline to write Python code. You'll find various tools to help you comply to the usual standards :
Also,
Use pythonic loops
Python provides you a clean/safe/concise/way to iterate over pretty much anything :
You can change :
to (using the default behavior for
then
then
then (because you don't need to create a temporary list) :
Don't use complicated non-required logic
Converting your tuple in a string you perform logic in in way too complicated on top of not being efficient.
You could just do something like (using what we call "tuple unpacking"):
This can even be more concise by doing the unpacking as part of the loop :
Using
You can use the
and you don't need to close the file manually.
Using the
It is usually a good habit to put your code behind a guard to have your
No need for
As pointed out in codesparkle's comment, you don't need to build a
Once this is done, you code looks like :
As a final note, your code could be written using a single call to
It make things hard to understand and to maintain. In your case, the simple way to do this is to feed your function
function() the parameters it needs.def function(letter_input, number_input, split_letters):Make your code easier to read
You can ensure you respect PEP 8, the guideline to write Python code. You'll find various tools to help you comply to the usual standards :
pep8, pylint, pyflakes, pychecker.Also,
function is a pretty bad function name, let's pick something better (for instance write_permutation).Use pythonic loops
Python provides you a clean/safe/concise/way to iterate over pretty much anything :
for something in something_iterable. Using the loop index is much more verbose on top of being less efficient.You can change :
all_combinations_list = list(itertools.permutations(split_letters, number_input))
number_of_combinations = len(all_combinations_list)
for n in range(0,number_of_combinations):
raw_characters = str(all_combinations_list[n])to (using the default behavior for
range)all_combinations_list = list(itertools.permutations(split_letters, number_input))
number_of_combinations = len(all_combinations_list)
for n in range(number_of_combinations):
raw_characters = str(all_combinations_list[n])then
all_combinations_list = list(itertools.permutations(split_letters, number_input))
for e in all_combinations_list:
raw_characters = str(e)then
for e in list(itertools.permutations(split_letters, number_input)):
raw_characters = str(e)then (because you don't need to create a temporary list) :
for e in itertools.permutations(split_letters, number_input):
raw_characters = str(e)Don't use complicated non-required logic
Converting your tuple in a string you perform logic in in way too complicated on top of not being efficient.
You could just do something like (using what we call "tuple unpacking"):
for e in itertools.permutations(split_letters, number_input):
a, b, c = e
out_2 = a+b+cThis can even be more concise by doing the unpacking as part of the loop :
for a, b, c in itertools.permutations(split_letters, number_input):
print >>output_file, a+b+cUsing
withYou can use the
with keyword to handle ressources that needs to be "opened" and "closed" such as files. In your case, you can write :with open('/tmp/list.txt', 'w') as f:
for a, b, c in itertools.permutations(split_letters, number_input):
print >>f, a+b+cand you don't need to close the file manually.
Using the
if__name__ guardIt is usually a good habit to put your code behind a guard to have your
main code running only if we run your script and not if we import it (because one wants to re-use a function for instance).No need for
list as strings are iterables tooAs pointed out in codesparkle's comment, you don't need to build a
list.Once this is done, you code looks like :
import itertools
def main():
"""Main function"""
split_letters = raw_input(' Type in letters: ')
number_input = int(raw_input(' Letters per word: '))
with open('/tmp/list.txt', 'w') as f:
for a, b, c in itertools.permutations(split_letters, number_input):
print >>f, a+b+c
if __name__ == "__main__":
main()As a final note, your code could be written using a single call to
print by using join on the list of permutations but this doesn't bring much so I'll skip that part.Code Snippets
def function(letter_input, number_input, split_letters):all_combinations_list = list(itertools.permutations(split_letters, number_input))
number_of_combinations = len(all_combinations_list)
for n in range(0,number_of_combinations):
raw_characters = str(all_combinations_list[n])all_combinations_list = list(itertools.permutations(split_letters, number_input))
number_of_combinations = len(all_combinations_list)
for n in range(number_of_combinations):
raw_characters = str(all_combinations_list[n])all_combinations_list = list(itertools.permutations(split_letters, number_input))
for e in all_combinations_list:
raw_characters = str(e)for e in list(itertools.permutations(split_letters, number_input)):
raw_characters = str(e)Context
StackExchange Code Review Q#54362, answer score: 6
Revisions (0)
No revisions yet.