Skip to content

Rendu Ewen Horville#29

Open
The17thDoctor wants to merge 5 commits intobcalou:mainfrom
The17thDoctor:main
Open

Rendu Ewen Horville#29
The17thDoctor wants to merge 5 commits intobcalou:mainfrom
The17thDoctor:main

Conversation

@The17thDoctor
Copy link
Copy Markdown

No description provided.

@The17thDoctor The17thDoctor marked this pull request as ready for review November 17, 2023 14:38
Copy link
Copy Markdown
Owner

@bcalou bcalou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bien. Certains algos pourraient bénéficier d'un peu plus de commentaires pour comprendre la mécanique.

Comment on lines +2 to +5
if number > 1:
return number * get_factorial(number - 1)
else:
return 1
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Utiliser un ternaire ?

Comment on lines +11 to +12
if i == 0:
continue
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On peut supprimer ça si on part directement du bon index ;)

return merge(sort(array[:middle]), sort(array[middle:]))
else:
# For array of size 2, just swap if needed
return [array[1], array[0]] if array[1] < array[0] else array
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Soit le comportement (simplifié) de la fonction merge pour ce cas précis. Il serait plus clair de rester sur la fonction merge plutôt que de répliquer ce comportement ici.

return [array[1], array[0]] if array[1] < array[0] else array


def merge(array_1: list[int], array_2: list[int]):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type de retour manquant

@bcalou
Copy link
Copy Markdown
Owner

bcalou commented Dec 5, 2023

Code clair et bien structuré. Quelques petites améliorations possibles (mentionnées en code review), mais très bien.
Quelques imprécisions dans les réponses sur la complexité.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants