2016-12-06 15 views
0

私は、多数の単語を取り、各単語を文字に分割し、それらを大きなリストに結びつけるループを持っています。発生ループの実行時間の短縮と効率の向上

list[0] =手紙:

私は、ほとんどが発生し、文字をチェックし、それがすでに文字列に表示されていない場合、私は2つのスペースがあり、リストに保存しますこのループは超非効率的である

の発生回数を最も

list[1]を=。それは動作しますが、値を返すのに約25〜30秒かかります。その前にはそれは続行し、値を返さないでしょう。

私が書いたコードの効率を上げるにはどうすればよいですか?

def choose_letter(words, pattern): 
    list_of_letters = [] 
    first_letter = [] # first spot is the letter, second is how many times it appears 
    second_letter =[] # first spot is letter, second how many times it appears 
    max_appearances = ["letter", 0] 
    for i in range(len(words)): # splits up every word into letters 
     list_of_letters.append(list(words[i])) 
    list_of_letters = sum(list_of_letters, []) # concatenates the lists within the list 
    first_letter = list_of_letters.count(0) 
    for j in list_of_letters: 
     second_letter = list_of_letters.count(j) 
     if second_letter >= max_appearances[1] and j not in pattern: 
      max_appearances[0] = j 
      max_appearances[1] = second_letter 
     else: 
      list_of_letters.remove(j) 
    return max_appearances[0] 
+2

これは、http://codereview.stackexchange.com/ – Blorgbeard

+1

のより良い候補かもしれません。そして、codereviewに移動すると、あなたはこのコードに対して実行したプロファイラの出力を見るように求められます。 –

+2

['collections.Counter'](https://docs.python.org/3.5/library/collections.html#collections.Counter)のジョブのようです。 – bereal

答えて

0

より速く進む1つの方法は、より良いデータ構造を選択することです。ここでcollections.Counterを使用した例です。

from collections import Counter 

def choose_letter(words, pattern): 
    pattern = set(pattern) 
    letters = (letter 
       for word in words 
       for letter in word 
       if letter not in pattern) 
    letters = Counter(letters) 
    return letters.most_common(1)[0][0] 


mywords = 'a man a plan a canal panama'.split() 
vowels = 'aeiou' 
assert choose_letter(mywords, vowels) == 'n' 

ここcollections.defaultdictを使用していずれかになります。

from collections import defaultdict 

def choose_letter(words, pattern): 
    pattern = set(pattern) 
    counts = defaultdict(int) 
    for word in words: 
     for letter in word: 
      if letter not in pattern: 
       counts[letter] += 1 
    return max(counts, key=counts.get) 

mywords = 'a man a plan a canal panama'.split() 
vowels = 'aeiou' 
assert choose_letter(mywords, vowels) == 'n' 
0

あなたは必要ありません&操作のリストをループをたくさんやっています。 countまたはnot inを実行するたびに、プログラムはリスト/文字列をループして、探しているものを見つけます。あなたのリストからそれらのアイテムをすべて削除することもかなり高価です。はるかにエレガントなソリューションは、単語/文字のリストを1回だけループし、辞書を使用して各文字の出現をカウントすることです。そこから、文字/カウントのペアを持つ辞書があります。&そこからキー/値を取得するだけで、リストをソートします。&最初の2つの値を見てください。

from collections import defaultdict 
from itertools import chain 

def choose_letter(words, pattern=""): 
    count_dict = defaultdict(int) # all unknown values default to 0 
    for c in chain(*words): 
     count_dict[c] += 1 
    # you could replace this "not in" with something more efficient 
    filtered = [(char, count) for (char,count) in count_dict.items() if char not in pattern] 
    filtered.sort(lambda a,b: -cmp(a[0], b[0])) 
    print filtered 
    return filtered[0][0] 

あなたは開梱引数に掘るしたくない場合は、& defaultdictsをitertools、あなただけ言うことができる:

count_dict = {} 
for word in words: 
    for char in word: 
     count_dict[char] = count_dict.get(char, 0) + 1 

...あなたは開梱引数を掘り下げるしようとしない場合まだ。

関連する問題