NicolasGlassey
commented
8 months ago Qualité du code
Rien de particuliers à mentionner.
Interfaces
Les interfaces respectent le cahier des charges :
- Vous annoncez (readme) utiliser la version 8.3 de Php qui supportent les types. Pourquoi ne pas les utiliser ?
Tests
- Setup permet de préparer la classe de test avant la séquence de test. Ok.
- Pourquoi avoir créé une méthode "clearBucket"... n'avez-vous pas un mécanisme déjà prévu dans PhpUnit pour faire ce travail ?
- Vous n'avez pas mentionné given, when, then... qui sont pourtant fait pour aider à la lecture du code. Si vous n'aimez pas cette approche, regardez au moins ce qui est prévu pour écrire des tests en BDD avec Php.
- Nombreuses répétitions qui auraient pu être codé dans une méthode appropriée de PhpUnit.
Implémentation
Gestion des variables
- Pourquoi contraindre votre objet Bucket à être instancié à l'aide d'un bucket name ?
- Un seul fichier .env pour les deux services.
- Différence d'implémentation entre DataObject (qui reçoit des paramètres) et le LabelDetector pour lequel il n'est pas possible de changer les credentials par le client.
Exceptions
- Vous devriez pas avoir besoin d'importer "Exception" dans vos classes de tests... pour cela il vous suffit de coder vos propres exceptions !
@NicolasGlassey
J'ai effectué les corrections sur les points auxquels vous m'aviez fait un retour. J'ai également mis à jour le point de la structure des dossiers dans le ReadMe même si vous aviez déjà fait la review, du coup c'est mon dernier commit
https://github.com/ThomasGrossmann/BI1/commit/24bfd0394471f8e8b8618fc74e470141f6e45d3e