-
Notifications
You must be signed in to change notification settings - Fork 0
Exercice 2 : Revue de revue #1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| """Modèle utilisant une base de donnée locale en JSON""" | ||
| def __init__(self): | ||
| # `utf-8` pour être sûr que le fichier est lu correctement quel que soit l'OS | ||
| with open("dsonames.json", "r+", encoding="utf-8") as f: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pourquoi ouvrir le fichier avec la permission r+, qui signifie lecture + écriture ? r, lecture seule, devrait être suffisant
| def names(self): | ||
| """Obtient la liste des noms, en anglais, des objets disponibles""" | ||
| result = [] | ||
| for _, planet in self.values.items(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| for _, planet in self.values.items(): | |
| for planet in self.values.values(): |
|
|
||
| def translated_names(self, language): | ||
| """Obtient une correspondance entre noms et noms traduits dans la langue donnée.""" | ||
| return [(p["name"], p[language]) for p in self.values.values() if language in p] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Est-ce normal que certaines planètes n'ont pas de nom traduit ? Devrions-nous le documenter ?
| def __init__(self, model, view): | ||
| """Crée un Presenter avec un modèle et une view donnés.""" | ||
| self.model = model | ||
| self.view = view |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_model et _view puisque ce sont des champs privés
|
|
||
| def _show_list(self, lst): | ||
| """Affiche la liste donnée via la view.""" | ||
| self.view.show("\n".join(["- " + str(s) for s in lst])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plutôt "* " pour une liste à puces, non ?
| class LocalDatabaseModelTests(TestCase): | ||
| def test_names(self): | ||
| m = LocalDatabaseModel() | ||
| self.assertIn("Sextans Dwarf Spheroidal Galaxy", m.names()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Devrions-nous tester l'ordre des planètes ?
| p = Presenter(FakeModel(), v) | ||
| self.assertEqual([], v.outputs) | ||
| p.run() | ||
| self.assertEqual(["Bonjour! Cette application vous donne des informations sur les groupes d'étoiles.", "- X\n- Y\n- Z"], v.outputs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Il existe des méthodes plus spécialisées que assertEqual, elles seraient utiles au cas où les tests échouent pour avoir des messages d'erreur plus clair
| @@ -0,0 +1,9 @@ | |||
| class ConsoleView: | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation ?
| self._show_list(self.model.names()) | ||
| return | ||
|
|
||
| parts = command.split(' ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| parts = command.split(' ') | |
| chunks = command.split(' ') |
| self._show_list(self.model.translated_names(parts[1])) | ||
| return | ||
|
|
||
| self.view.show("Commande inconnue") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pas génial
Quels commentaires sont utiles ? Lesquels ne le sont pas ? Manque-t-il des commentaires ?
Ouvrez l'onglet "Files changed" pour commencer