patternpythonMinor
Python comparing a string or list of strings to a string or list
Viewed 0 times
comparingpythonliststringsstring
Problem
I'm using a selfmade python program for learning new words in new languages. Originally I made this program for a university course but now I'm just trying to imrove it and make the code "prettier" and perhaps some small functionality improving. :)
The basic principle is that the program reads from a file the foreign word and the native word, saves them into two lists ([foreign_words] and [native_words]). If I remember the word correct, it will remove the word from both lists and ask a random new word from the lists. If I fail to remember it correctly, it will not remove it, and it will copy the words so that it'll have to ask the word twice more. The actual code is written in my native language but I'll try to translate most of it. Also the actual program is quite long so I will try to include only the relevant parts of it.
The actual files are in format:
and the words will be saved to their respective lists. This below is how I read the files. Just for reference. It works and I don't think I really need to change anything there.
Now for the part in my program that I wish to improve. I feel like it's really not too effective code even though it mostly does what it's supposed to. I use isinstance(correct_word, str) to check if the word is a single word (not separated by a comma in the file) or multiple words (separated by commas in the file). But I'm struggling to find a proper
The basic principle is that the program reads from a file the foreign word and the native word, saves them into two lists ([foreign_words] and [native_words]). If I remember the word correct, it will remove the word from both lists and ask a random new word from the lists. If I fail to remember it correctly, it will not remove it, and it will copy the words so that it'll have to ask the word twice more. The actual code is written in my native language but I'll try to translate most of it. Also the actual program is quite long so I will try to include only the relevant parts of it.
The actual files are in format:
foreign_word:native_word, another_wordand the words will be saved to their respective lists. This below is how I read the files. Just for reference. It works and I don't think I really need to change anything there.
for line in open(file, "r", encoding="UTF-8"):
if ":" in line:
(foreign, native) = line.rstrip("\n").split(":")
if "," in foreign:
foreign_words += [foreign.split(",")]
else:
foreign_words += [foreign]
if "," in native:
native_words += [native.split(",")]
else:
native_words += [native]
self.__foreign_words = foreign_words
self.__native_words = native_words
x = random.randint(0, (len(self.__foreign_words) - 1))
self.__index.set(x)Now for the part in my program that I wish to improve. I feel like it's really not too effective code even though it mostly does what it's supposed to. I use isinstance(correct_word, str) to check if the word is a single word (not separated by a comma in the file) or multiple words (separated by commas in the file). But I'm struggling to find a proper
Solution
In your file reading part:
Use the with .. as construct
Use
This will ensure that the file will be closed at the end.
split is more awesome than you think
So there is no need to check whether there is a "," in there. Note that both this and your original code will not strip the spaces after ":" and ",", if you enter for example:
So you might have to do something like this at the end:
You can also directly assign the read foreign_words to the class, no need for the intermediary list:
From the code you posted I don't see any reason to have this in the file reading part:
Because in the second part of the code you immediately set x as well.
In your second part:
Can be simplified to
Instead of putting everything in an if block to make sure there are words to process, you could do:
join is more awesome than you think
So no need to check for
But I would not do this, because if you leave your correct_word as a list of correct words (possibly containing only one word, which is fine), then checking for the right answer becomes what you want:
When checking whether the list is still not empty, it is better to set L to the new length:
Instead of
This way you can wrap this in a while loop more easily and repeat until the user quits or has finished all words. Your current code seems to only support asking for two words in a row.
Use the with .. as construct
Use
with open(file, "r", encoding="UTF-8") as words_file:
for line in words_file:This will ensure that the file will be closed at the end.
split is more awesome than you think
>>> "abc".split(",")
["abc"]So there is no need to check whether there is a "," in there. Note that both this and your original code will not strip the spaces after ":" and ",", if you enter for example:
foreign_word: native_word1, native_word2So you might have to do something like this at the end:
foreign = [word.strip() for word in foreign]You can also directly assign the read foreign_words to the class, no need for the intermediary list:
self.__foreign_words.append(foreign.split(","))
self.__native_words.append(native.split(","))list.append() is also faster than list += other_listFrom the code you posted I don't see any reason to have this in the file reading part:
x = random.randint(0, (len(self.__foreign_words) - 1))
self.__index.set(x)Because in the second part of the code you immediately set x as well.
In your second part:
if language == "F":
correct_word = self.__native_words[x]
elif language == "N":
correct_word = self.__foreign_words[x]Can be simplified to
# In the class add this member
self.dictionary = {"F": self.__native_words, "N": self.__foreign_words
...
# Then you can just use later
correct_word = self.dictionary[language][x]Instead of putting everything in an if block to make sure there are words to process, you could do:
assert(L>0)
# or
if L==0:
exit() # or continue if you are in a loop
# Rest of codejoin is more awesome than you think
>>> ", ".join(["abc"])
'abc'So no need to check for
isinstance(words,str), just use:self.__question.set(", ".join(correct_word))But I would not do this, because if you leave your correct_word as a list of correct words (possibly containing only one word, which is fine), then checking for the right answer becomes what you want:
if answer in correct_words:
# Do stuffWhen checking whether the list is still not empty, it is better to set L to the new length:
L = len(self.__foreign_words)
if L > 0:
# Do stuffInstead of
if L - 1 > 0:
# Do StuffThis way you can wrap this in a while loop more easily and repeat until the user quits or has finished all words. Your current code seems to only support asking for two words in a row.
Code Snippets
with open(file, "r", encoding="UTF-8") as words_file:
for line in words_file:>>> "abc".split(",")
["abc"]foreign_word: native_word1, native_word2foreign = [word.strip() for word in foreign]self.__foreign_words.append(foreign.split(","))
self.__native_words.append(native.split(","))Context
StackExchange Code Review Q#133579, answer score: 3
Revisions (0)
No revisions yet.