yalqaoud / pwp-capstones

0 stars 0 forks source link

Change two small things in frequency_comparison #5

Open jmcrey opened 5 years ago

jmcrey commented 5 years ago
def frequency_comparison(table1, table2):
    appearances = 0
    mutual_appearances = 0

    for x in table1:
        for y in table2:
            if x == y:
                if x < y:
                    mutual_appearances += table1[x]
                    appearances += table2[y]
                else:
                    mutual_appearances += table2[y]
                    appearances += table1[x]
            else:
                appearances += table1[x]

    for y in table2:
        if y not in table1:
            appearances += table2[y]

    return mutual_appearances / appearances

This is a great implementation! But, to simplify this function, we can do two things. First, we can change the for loop to an if statement:

if x in table2.keys()

This way, we simply test if the value exists in table2 in one line instead of iterating through each of the values in table2. Basically, the if statement will stop once (or if) we find the correct key in the second table; but, the original for loop implementation will not stop after it finds the correct value -- it will continue through each of the keys per iteration regardless. Thus, the if statement may be more efficient.

Also, after we change this line, we can actually completely remove the if statement after it:

if x == y

Essentially, this line can be removed because it no longer has value -- we already know there is a key that equals x in the second table because of the if statement.

Of course, the first implementation will definitely work; it can just be simplified a little bit. Hope this helps!