HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Python comparing a string or list of strings to a string or list

Submitted by: @import:stackexchange-codereview··
0
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:

foreign_word:native_word, another_word


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.

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

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_word2


So 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_list

From 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 code


join 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 stuff


When 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 stuff


Instead of

if L - 1 > 0:
    # Do Stuff


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.

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_word2
foreign = [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.