def jump_to(self, position_piece, target_position):
middle_position = Position((position_piece.line + target_position.line)/2,
(position_piece.column + target_position.column)/2)
if position_piece not in self.cases:
return False
elif (target_position not in position_piece.four_position_jumps() and
target_position in self.cases):
return False
elif middle_position in self.cases:
if self.cases[position_piece] == self.cases[middle_position]:
return False
return True
I have created the above function, but I realised I returned 3 times False. It looks bad implementation for me. Is it a fine implementation? How can I modify that code to make it more pythonic? I would be interested to know how to return False once instead of three.
3 Réponses :
Cela pourrait être un autre moyen, même si je ne dirais pas qu'il est beaucoup plus lisible ou intuitif et j'ai également écrit le code sur une forme plus minimaliste pour l'exemple: Remarque: car Vous vérifiez que cible_position dans Self.cases: Le comparateur et signifie que la vérification de cela suffit à couvrir les cas.
Ceci n'est clairement pas plus lisible que l'original.
@Glennmackintosh Ceci est clairement signalé ... La question était d'utiliser une déclaration false .
Vrai, tu as fait remarquer ça. En ce qui concerne la question, j'ai interprété l'objectif de l'OP pour être la partie où il a demandé est-ce une belle mise en œuvre? et il est plus clair et aussi meilleur que les alternatives. Vous avez dit que le vôtre n'était pas beaucoup plus lisible, mais je ne voudrais pas être en désaccord et dire que le vôtre est nettement moins lisible.
@GlennMackintosh suppose que c'est la beauté de la programmation. De nombreuses solutions pour un problème - et une préférence personnelle impliquée. Merci de votre avis, très constructif, j'ai tellement appris.
Je suis d'accord avec le commentaire de @ LeopardxProLoad en ce que les multiples false code> renvoie ne sont pas le principal problème avec votre code, en particulier car il ne dispose que d'un vrai code>. Je vois votre calcul de Middle_position Code> tôt dans le code, juste pour l'ignorer dans certaines situations, comme un problème plus important: def jump_to(self, position_piece, target_position):
if position_piece not in self.cases:
return False
if target_position in self.cases and \
target_position not in position_piece.four_position_jumps():
return False
line = (position_piece.line + target_position.line) / 2
column = (position_piece.column + target_position.column) / 2
middle_position = Position(line, column)
if middle_position in self.cases:
if self.cases[position_piece] == self.cases[middle_position]:
return False
return True
Je quitterais probablement le si Middle_position dans Self.cases Pré-Vérifiez comme si vous aviez initialement avant la modification, mais enveloppez l'enregistrement dans un essai / sauf pour l'attraper lorsque Middle_position n'est pas dans Self.cases . Cela ressemble à un dictionnaire pour que vous voudriez attraper le KeyError.
Changer le elif s sur si s est meilleur. Cependant, il me manque peut-être quelque chose ici, mais il semble que votre code est incorrect, car vous avez laissé tomber le cible_position dans Self.cases Vérifiez à partir du deuxième test. Il n'est pas clair pour moi comment cible_position non dans la position_piece.four_position_jumps () incorpore une partie de la condition et ed.
@Glennmackintosh, oui, j'ai accidentellement laissé tomber cette clause que j'ai à l'origine combiné avec son duplicata (avant de poster) qui disparaissait dans la modification de l'OP. Bien que lorsque lorsque vous l'ajoutez à l'heure actuelle, je l'ai renversé car il semble plus efficace de faire le dans et ensuite l'appel de fonction si le dans échoue. Je n'aime pas travailler sur le code je ne peux pas tester juste pour cette raison!
Votre implémentation est décente mais pourrait être améliorée lors de l'utilisation de La réponse de @cdlane.
Cette réponse vise à utiliser un seul point de sortie. Bien que cela puisse être plus lisible, la réponse de CDLANE. Il est plus complexe et probablement moins maintenu, puis la réponse de CDLane, et vous devez utiliser sa réponse à la place. Mais voici comment une seule déclaration pourrait ressembler.
Il suffit de définir une variable appelée quelque chose comme le résultat = true, puis changez-le résultat = false dans la déclaration IF et au résultat de la déclaration de fin.
Pour être honnête, la façon dont votre code est disposé est lisible - sans cesse de manière significative si des déclarations.
Bien que vous ayez un certain nombre de conditions pour tester que tout retour Faux, il est relativement clair et facile à suivre de cette façon. Vous pouvez le combiner dans un seul
si condition, mais ce ne serait probablement pas plus facile à suivre. La grande raison pour laquelle c'est probablement mieux c'est parce qu'ils font juste un retour. S'ils étaient chacun exécutant les mêmes lignes de code multiples qui devaient être copiées dans chacune (avec la possibilité d'erreur si une modification était nécessaire), ce serait une situation différente.