问题描述
我需要帮助,除了帮助我之外,人们还告诉我,我的代码虽然可以正常工作,但完全是一团糟,属于“意大利面条代码”类别。
有人可以协助我如何重构代码以提高效率和可读性吗?
非常感谢您能给我的帮助。
这是我的代码:Python上的Hangman游戏
def setup(app):
app.add_directive("mydirective",MyDirective)
app.add_role("myrole",MyRole())
解决方法
我偶然发现
secretWord = random.sample(words,1)
我期望secretWord
是一个单词(即字符串)。但是,您始终使用[0]
访问它。我不得不在调试器中查看,secretWord
是一个包含1个单词的列表。这违反了Clean Code的“最少惊讶的原则”和“不要重复自己”(您需要在多个位置重复索引器[0]
)。
numberOFLetters
应该是numberOfLetters
。
您已经计算出字母的数量,但是没有一致地使用它。 wordSpaces = len(secretWord) * '_'
可能是wordSpaces = numberOfLetters * '_'
。
使用与计算wordSpaces = numberOfLetters * '_'
相同的方法,您可以摆脱循环打印下划线:
for char in secretWord:
print('_ ',end='')
可能是
print(numberOfLetters * '_ ')
你有
listedWordSpaces = list(wordSpaces)
其中wordSpaces
是包含下划线的字符串。字符串已经是char
的列表。对我来说,这听起来像是重复的,违反了KISS(“保持简单而愚蠢”)和YAGNI(“您将不需要它”)的原则。实际上,似乎决定会迫使您编写更多代码:def converter(list):
是反向操作。
顺便说一句,这不仅是一个函数,而且还有一个副作用:它将打印字符串。并且从不使用返回值。而且,该方法比所需的更为复杂。 initialStr = ''.join(list)
应该给出相同的结果。
def char_positioner(word,guessAttempt):
中有类似的打印副作用。使用时的变量名是position = char_positioner(secretWord,guess)
,我希望position
是int
,但显然它是一个字符列表。令人困惑。
您在此行违反了PEP 8:
misses = 0
因为在函数定义之后应该有2个换行符。
还有一个PEP 8问题,这里的运算符之间存在间距:
print('You lose! - The word was:' + '"' + secretWord[0] + '"')
,最后可能缺少换行符(不过可能是SO复制/粘贴问题)。
您还有几个shadowing issues:
def converter(list): [...]
for char in list: [...]
def getGuess(guess): [...]
我还在这里看到DRY(“不要重复自己”)问题:
if misses == 1:
hangmanOne()
[...]
想象一个更复杂的子手。您会写100个方法和100个if / else语句吗?
某些函数名称的名称类似于名词,但应改为动词:
def converter(list): [...]
def char_positioner(word,guessAttempt): [...]
我个人想看看type hints,所以我可以更好地理解参数的预期类型是什么以及返回类型是什么。
我看到了while game == 'on':
。通常,我们在这里不使用字符串,而是布尔值。似乎也没有办法通过转动"off"
来停止游戏。退出案例是通过break
实现的,因此目前可能是while True
循环。随着游戏的发展,break
语句有些脆弱,可能不再脱离最外层的循环。像while isPlaying
或while not isGameOver
这样的布尔条件可能是更好的选择。