Rock - Delia #73
Rock - Delia #73
Conversation
There was a problem hiding this comment.
Awesome job Delia!! You hit all the learning goals and completed all the user stories for this project. Overall, your code was well-organized and easy to read. I've added some comments for refactoring and ways to make your logic more robust 💪
Overall, great job and keep up the hard work 🔥 ✨
|
|
||
| if video == None: | ||
| return "This video has not been found" | ||
|
|
||
| elif video.isalpha(): | ||
| response = video_store.list_one_video(title=video) | ||
| elif video.isdigit(): | ||
| response = video_store.list_one_video(id=int(video)) |
There was a problem hiding this comment.
A good edge case to handle is invalid inputs? Providing some logic that will notify the user about invalid data being entered and then asking to 'Enter a video title or id' again would be really helpful and improve the overall ux of the cli.
| return "This customer has not been found" |
There was a problem hiding this comment.
This particular logic could also expand out to invalid data.
| if customer == None: | |
| return "This customer has not been found" | |
| if customer == None: | |
| return "This customer has not been found" |
|
|
||
| run_cli() | ||
|
|
There was a problem hiding this comment.
Delia! Your main.py file is really neat and easy to read! Great job in separating out the requests from this menu logic.
| def make_choice(options): | ||
| valid_choices = options.keys() | ||
| choice = input("\nPlease select option: ") |
There was a problem hiding this comment.
Adding some detail on how the user can see all the options would be really helpful as the previously displayed menu can get pushed up after doing a few commands.
| choice = input("\nPlease select option: ") | |
| choice = input("\nPlease select option or enter 15 to see all options again: ") |
| self.selected_video = None | ||
| return response.json() |
There was a problem hiding this comment.
Adding some logic to check if the response returns a "video not found" and then printing a message that describes the video not being found would be really helpful to the user.
There was a problem hiding this comment.
Currently I can type in an invalid video and/or customer and the CLI crashes.
| response = requests.get(self.url + f"/videos/{video_id}/rentals") | ||
| print(response.text) | ||
| return response.json() | ||
|
|
There was a problem hiding this comment.
Great job in separating your requests from main.py. In a future refactor, you could separate customer, rental, and video requests into different files to further modularize your code 😄
No description provided.