2011-01-12 11 views
2

私は最近/usr/share/dict/wordsを回り、私のispalindrome(x)メソッド を使って回文のリストを返しました。ここにいくつかのコードがあります...何が間違っていますか?それだけで10分間ストールした後、pythonとpalindromes

 
def reverse(a): 
    return a[::-1] 

def ispalindrome(a): 
    b = reverse(a) 
    if b.lower() == a.lower(): 
     return True 
    else: 
     return False 

wl = open('/usr/share/dict/words', 'r') 
wordlist = wl.readlines() 
wl.close() 
for x in wordlist: 
    if not ispalindrome(x): 
     wordlist.remove(x) 
print wordlist 
+2

'ispalindrome = lambda a:a [:: - 1] .lower()== a.lower()'という短いインライン定義は、〜25%の時間を節約します。 – eumiro

+4

あなたの問題とは無関係ですが、2番目の関数は 'return(reverse(a).lower()== a.lower())'に減らすことができます。 '=='はすでに 'True'または' False'を返すので、 'if'文をその周りにラップする必要はありません。 –

答えて

5
wordlist = wl.readlines() 

は、最後に改行文字があるので、あなたのリストは次のようである:の

['eye\n','bye\n', 'cyc\n'] 

要素が明らかに回文ではありません。

あなたはこの必要があります。

['eye','bye', 'cyc'] 

のでstrip改行文字を、それは問題ないはずです。

wordlist = [line.strip() for line in open('/usr/share/dict/words')] 

はEDIT:リストの繰り返し処理し、それが問題を引き起こしている修正1行でこれを行うには

Matthewで指摘されているリストの理解を使用してください。

1

あなたは/usr/share/dict/words内の各単語の末尾の改行を含めているファイル内のすべての単語のリストを返します。つまり、あなたは決して回文を見つけることはできません。回文以外のものをリストから削除するのではなく、見つかったときに回文を記録するだけで、スピードアップします。これを行うと

+0

これは当てはまりますが、何が印刷されるのかは説明されていません。 –

+0

@Johns、元のコードを読んでください。それは、* non * -palindromesを削除しようとします。そしてそれは半分を削除します。反復中に変更することですべてを削除することはできません。 –

+0

あなたはもちろんです。私はそれに応じて私の答えを更新しました。 – Johnsyweb

3

私は2つの問題があると思います。

まず、すべての単語をリストに読み込む際のポイントは何ですか?各単語を順番に処理して、それが回文の場合は印刷しないでください。

次に、空白に注意してください。 wordの末尾に改行があります。

あなたは(空白のために)回文を特定していないので、リストからすべてのアイテムを削除しようとします。あなたがそれを反復している間に!

このソリューションは、第二の下にウェル内で実行され、回文たくさんの識別:

for word in open('/usr/share/dict/words', 'r'): 
    word = word.strip() 
    if ispalindrome(word): 
     print word 

編集を:

def ispalindrome(a): 
    return a[::-1].lower() == a.lower() 

words = (word.strip() for word in open('/usr/share/dict/words', 'r')) 
palindromes = (word for word in words if ispalindrome(word)) 
print '\n'.join(palindromes) 
+0

しかし、奇妙なのは、リストでないアイテムがリストから削除されているということです。そのため、すべてのアイテムを削除する必要があります。 –

+0

@ティム:ああ...はい。あなたが正しい。これはリストから*すべてを削除しようとしますが、反復しています。不快な! – Johnsyweb

2

おそらくより 'ニシキヘビ' generator表現を使用することです

すべての単語を返しません。それは半分を返します。これは、反復処理中にリストを変更しているためです。これは間違いです。より簡単で効果的なソリューションは、リストの理解度を使用することです。あなたは、全体のことを行うためにスクバー年代を変更することができます:あなたはまた、これを破ることができる

[word for word in (word.strip() for word in wl.readlines()) if ispalindrome(word)] 

stripped = (word.strip() for word in wl.readlines()) 
wordlist = [word for word in stripped if ispalindrome(word)] 
3

その他はすでに、より良い解決策を指摘しています。私はあなたのコードを実行した後にリストが空でない理由を示したいと思います。 ispalindrome()関数はTrueを返しません。これは、他の回答に記載されている「改行問題」のため、あなたのコードはすべての単一項目に対してwordlist.remove(x)を呼び出します。では、なぜリストは最後に空ではないのですか?

リストを反復処理しているので、リストを変更しているためです。次のことを考えてみましょう:

>>> l = [1,2,3,4,5,6] 
>>> for i in l: 
...  l.remove(i) 
... 
>>> l 
[2, 4, 6] 

あなたが1を削除すると、残りの要素が上向きに一歩を移動するので、今l[0]2です。ただし、反復カウンタは進化しており、次の反復でl[1]が表示されるため、3などを削除します。

あなたのコードでは、エントリの半分が削除されます。道徳:あなたが何をやっているのか正確にわからない限り)反復している間はリストを変更しないでください:)。