Skip to content

Solution#3734

Open
jlaourayed wants to merge 1 commit intomate-academy:masterfrom
jlaourayed:develop
Open

Solution#3734
jlaourayed wants to merge 1 commit intomate-academy:masterfrom
jlaourayed:develop

Conversation

@jlaourayed
Copy link
Copy Markdown

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

Excellent work on this! Your solution correctly implements the count_occurrences function and passes all requirements, so I am approving it. You did a great job handling the case-insensitivity requirement by converting both inputs to lowercase.

For future reference, you can make your code a bit more concise. Instead of converting the string to a list, you can call the .count() method directly on the string itself. This would allow you to simplify your function into a single line: return phrase.lower().count(letter.lower()).

Also, a small tip on coding conventions: using English for variable names is standard practice and makes code easier for a wider audience to read. These are just suggestions for improvement—your code works perfectly. Keep up the great work!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

Comment thread app/main.py
Comment on lines +2 to +3
phrase_en_liste = list(phrase.lower())
nombre_occurence = phrase_en_liste.count(letter.lower())
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

While this solution works perfectly, the conversion of the phrase to a list is an unnecessary intermediate step. Python's string objects have their own count() method, which you can use directly on the lowercased string. You could combine lines 2, 3, and 4 into a single return statement for a more concise solution.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fixed in 5a3a660. Added ge=0 on offset and ge=1, le=5000 on limit via Field() constraints.

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.

3 participants