Conversation
bcalou
left a comment
bcalou
left a comment
There was a problem hiding this comment.
Quelques amélirations possibles mais bonne direction
doomsday/date.py
Outdated
| # Vérifier si le nombre de caractère est bon (voir ci-haut) | ||
| if len(year) < 4: | ||
| print("Year must have 4 or more characters") | ||
| return False |
There was a problem hiding this comment.
Si l'année est supérieure à 1583, il y a forcément 4 caractères ou plus, n'est ce pas ?
doomsday/date.py
Outdated
| if len(month) < 1 or len(month) > 2: | ||
| print("Month must have 1 or 2 characters") | ||
| return False |
There was a problem hiding this comment.
Dans la même logique, on peut directement regarder la valeur du mois pour s'assurer que le nombre de caractères est correct
There was a problem hiding this comment.
on peut en effet vérifier que le mois est compris entre 1 et 12 (donc 1 ou 2 caractères), cependant "0001" sera accepté alors qu'il y a 4 caractères. je mets en commentaires la partie correspondante
doomsday/algorithm.py
Outdated
| days_of_week = ( | ||
| 'Sunday', | ||
| 'Monday', | ||
| 'Tuesday', | ||
| 'Wednesday', | ||
| 'Thursday', | ||
| 'Friday', | ||
| 'Saturday' | ||
| ) |
There was a problem hiding this comment.
Généralement le genre de choses qu'on met en constante (DAYS_OF_WEEK) en haut du fichier plutôt que dans une fonction
doomsday/algorithm.py
Outdated
| while multiple_of_7 % 7 != 0: | ||
| multiple_of_7 += 1 |
There was a problem hiding this comment.
Possible à calculer sans boucle while
doomsday/algorithm.py
Outdated
| match (century % 4): | ||
| case 0: # Dates 1600, 2000, etc. | ||
| century_value = 2 | ||
| case 1: # Dates 1700, 2100, etc. | ||
| century_value = 0 | ||
| case 2: # Dates 1800, 2200, etc. | ||
| century_value = 5 | ||
| case 3: # Dates 1500, 1900, etc. | ||
| century_value = 3 |
There was a problem hiding this comment.
Un match dont les cases sont des chiffres allant de 0 à 3 ressemble fortement à simple recherche dans un tableau [2, 0, 5, 3]
doomsday/algorithm.py
Outdated
| case 11: | ||
| return days_of_week[(day + anchor_day) % 7] | ||
| case 12: | ||
| return days_of_week[(day - 5 + anchor_day) % 7] |
There was a problem hiding this comment.
Cette opération est effectuée une douzaine de fois, or seul le chiffre au milieu change ! Il faut éviter cela et d'abord obtenir le chiffre, puis faire l'opération une seule fois.
Le match est une structure très utile mais pas forcément efficace ici
|
Code soigné mais 2 tests sur 3 ne passent pas, ce qui est essentiel. |
2 participants
No description provided.