ロジックを非常に複雑にしすぎて、サイズを小さくして何が間違っているかを示すために、いくつかのパスを作成する必要があります。
最初に、他の人が指摘したように、実際に報告されたエラーを修正します。同時に、単純な原則を適用します。ブールリテラルと比較しないでください。「雨が降っているのなら傘が必要だ」とは言わないでください。「雨が降っているなら傘が必要です」とおっしゃっています。だから余分なものを切り取ります。if isValid
は、何を意味するのかを正確に強調しているため、よりも明確です。また、デバッグトレース(コードが正しいことを実行しているかどうかを確認するためにのみ存在するステートメント。最初にコードを単純化してから、確認するものが少なくなります)も取り出します。if isValid == True
isValid
print
def match(pList):
b = []
z = len(pList)-1
for x in range(z):
b.append([pList[x][0],0])
for x in range(z):
isValid = False
q = p = 0
while not isValid:
q = random.randint(0, z)
if q > z:
isValid = False
elif q < 0:
isValid = False
elif pList[q][1]:
isValid = False
else:
isValid = True
isMatch = False
while not isMatch:
if not pList[q][1]:
isValid = False
while not isValid:
p = random.randint(0,z)
if p > z:
isValid = False
elif p < 0:
isValid = False
elif pList[p][2]:
isValid = False
else:
if q == p:
isValid = False
else:
isValid = True
b[q][1] = pList[p][0]
isMatch = True
return b
次に、条件付きロジックを単純化します。まず第一に、から返される結果は、何があっても、決してそうではありrandom.randint(0, z)
ません。それはまさに関数のポイントの一部です。したがって、これらのケースを処理するコードを作成する意味はなく、実際にそうするのは誤りです。何かを処理するコードを書くことは、それが実際に起こる可能性があることを意味します。それはコードを読んでいる人にとって気を散らすものであり、それは嘘です。重要なもの(呼び出しと値のチェック)の間に余分なスペースを置きます。< 0
> z
random.randint
pList
また、それに応じて別のブール値を設定するif/elseペアを単純化します。あなたが書かないのと同じ理由で
if x == 1:
y == 1
elif x == 2:
y == 2
# ... etc. ad infinitum for every possible integer value of x
ブール値でも同じことをするべきではありません。最後に、論理積and
とor
ブール条件を接続するために使用でき、使用する必要があります。
def match(pList):
b = []
z = len(pList)-1
for x in range(z):
b.append([pList[x][0],0])
for x in range(z):
isValid = False
q = p = 0
while not isValid:
q = random.randint(0, z)
isValid = not pList[q][1]
isMatch = False
while not isMatch:
if not pList[q][1]:
isValid = False
while not isValid:
p = random.randint(0,z)
isValid = not pList[p][2] and (q != p)
b[q][1] = pList[p][0]
isMatch = True
return b
次のステップは、リストのインデックスを修正することです。リストへのインデックス作成は、通常、実際には必要なものではなく、実際、ここではバグが発生します。pList
;の各「行」を反復処理する必要があることは明らかです。ただし、からまでのrange(z)
数値が得られるため、の計算でから1を引くのは正しくありません。たとえば、が5つの要素である場合、を計算して生成します。にアクセスすることはなく、4つの要素しかありません。0
z-1
len(pList)
z
pList
z = 4
range(z) = [0, 1, 2, 3]
pList[4]
b
z
基本的に、あなたは2つのことをしています。1つは、に「行」がある回数だけループを実行しpList
、(最初のループで)各「行」で何かを実行することです。これをする
これは非常に重要です。range
魔法ではなく、forループとの特別な関係はありません。これは、数値のリストを生成する単なる関数です。Pythonでは、for
ループは要素を直接提供します。このすべてのインデックス付けのナンセンスはまさにそれです-ナンセンス、それは能力の低い言語に任せるのが最善です。リストの各要素で何かをしたい場合は、リストの各要素をループするコードを記述して、リストの各要素で何かを行います。直接。元のインデックスに戻すために使用するインデックスの個別のリストを超えないようにします。それは物事を複雑にするためにあなたの邪魔にならないでしょう。
次に行うことはz
、可能なインデックスである乱数を生成することです。これにより、インデックスを作成しpList
てランダムな行を取得できます。つまり、ランダムな行を選択するだけです。したがって、ランダムな行を選択するだけです。このrandom
モジュールは、この機能を直接提供します。この関数はと呼ばれrandom.choice
、そのように聞こえます。
ここに1つの小さな問題があります。元のコードは比較p == q
します。つまり、ランダムに選択された2つのリストインデックスが等しいかどうかを比較します。インデックスを作成しなくなった場合、比較するインデックスはありません。これを修正するには、元の目的が何であったかを理解する必要があります。つまり、新しく選択された行が実際に古い選択された行であるかどうかを確認することです。繰り返しになりますが、これを直接チェックすることで単純化します。新しいインデックスではなく新しい行を選択し、それis
が古い行かどうかを確認します。
また、以前にとして識別しb
た行に対応する行を選択する必要があるという問題もあります。これに対処するために、行と同時に行を選択することができます。これは少し注意が必要です。私のアプローチは、行のペアのリストを作成することです。各ペアには、からの行があり、次にからの行があります。これには複雑なことは何も必要ありません。実際には、必要に応じて正確につなぎ合わせる組み込み関数があります。これはと呼ばれます。とにかく、行ペアのリストから行ペアを選択したら、2つの行を2つの変数に解凍する必要があります-を使用してpList
q
b
pList
b
pList
b
pList
zip
q, p = ...
結局のところ、最初に誤って使用していた構文。それが目的です。
p
これらの変更により、実際に、q
そしてz
完全に取り除くことができます。とにかくそれらの名前が何を意味するのかがまったく明確ではないので、これは素晴らしいことです。
def match(pList):
b = []
for row in pList:
b.append([row[0], 0])
for row in pList:
isValid = False
while not isValid:
first_row, b_row = random.choice(zip(pList, b))
isValid = not first_row[1]
isMatch = False
while not isMatch:
if not first_row[1]:
isValid = False
while not isValid:
second_row = random.choice(pList)
isValid = not second_row[2] and (first_row is not second_row)
b_row[1] = second_row[0]
isMatch = True
return b
もう少し論理的なクリーンアップの時間です。isValid
最初のwhileループでは、 trueになるまでループを続けます。つまり、not first_row[1]
真になるまで。2番目のwhileループでfirst_row
は、変更されることはありません。したがって、not first_row[1]
ループが開始されたときにtrueであったため、常にtrueのままになります。したがって、そのif-checkは完全に不要です。
それがなくなると、2番目のwhileループも実際には完全に役に立たないことがわかります。while not isMatch
つまり、まで isMatch
ループします。何isMatch
ですか?さて、ループを開始する前は、False
であり、ループの終わりでは、True
です。いつも。したがって、このコードは1回だけ実行されることがわかります。ループに入り、最後に移動し、isMatch
trueに設定してから終了します。これisMatch
は、trueに設定されたばかりのがtrueであるためです。一度だけ実行されるコードはループを必要としません。それはただのコードです。
ここで行うもう1つのことは、ループが完了したら、ループをアウトにwhile isValid
変換して、ループを少しクリーンアップするbreak
ことです。悪ではありません(そしてどちらもbreak
悪ではありませんcontinue
)。これ以上チェックしないため、ブール値についての考え方が実際に単純化not isValid
されます(強調not
)。に割り当てたものと直接比較しているだけですisValid
。isValid
これは、変数も削除することを意味します。これも、実際にはあまりわかりません。
def match(pList):
b = []
for row in pList:
b.append([row[0], 0])
for row in pList:
while True:
first_row, b_row = random.choice(zip(pList, b))
if not first_row[1]:
break
while True:
second_row = random.choice(pList)
if not second_row[2] and (first_row is not second_row):
break
b_row[1] = second_row[0]
return b
b
最後にもう1つ、よりクリーンに構築できます。要素を追加してリストを作成するのは、やっかいなゲームです。リストの作成方法をPythonに教えないでください。それは方法を知っています。代わりに、リスト内包表記を使用して、仕様に一致するリストを要求してください。これは説明よりも簡単に示されているので(説明が必要な場合はGoogleに相談してください)、先に進んでもう1つのバージョンを提供します。
def match(pList):
b = [[row[0], 0] for row in pList]
for row in pList:
while True:
first_row, b_row = random.choice(zip(pList, b))
if not first_row[1]:
break
while True:
second_row = random.choice(pList)
if not second_row[2] and (first_row is not second_row):
break
b_row[1] = second_row[0]
return b
これからは、実際に何をしているのかを理解せずに、何かを修正したり改善したりすることは困難です。このすべての作業の後、私はまだわかりません!
設定方法では、行の数だけランダムな行を選択しますが、重複を選択することもできます。それはあなたが本当に欲しいものですか?または、各行をランダムな順序で1回選択しますか?とにかく、ランダムな順序でそれを行うことの重要性は何ですか?
最初の行を選択した後、一致するランダムな2番目の行を選択します。最初の行ごとに1つのランダムな行が本当に必要でしたか?または、可能なすべての行のペアを試したいですか?
そして、とにかく、このデータのすべては何ですか?出力b
データは何を表していますか?そもそも何が入っpList
ているのか、なぜそれが呼ばれるのpList
か?match
この機能と「一致」しているのは何ですか?正直想像できません。