Code Review Comments (Robin Tun)

[robintun]

Properly Formatted Here: https://docs.google.com/document/d/1cVCBWqQH7n8HQwecNEdxX0xvkgp-FDhT6DsJs-Gj_sU/edit?usp=sharing

playlists.py

1. For the add_song_to_playlist method, it would be convenient for the users (leading to a better user experience) to be able to add multiple songs at once into a playlist instead of being able to add only one at a time.

• To implement this, I would suggest using some sort of loop and a data structure (maybe a list) to be able to add multiple songs at once into a playlist.

2. I am not sure if the CreatePlaylist, PlaylistIdResponse, and Song classes are necessary because they only have one attribute that is associated with them.

• I would remove these all together, and instead of passing in new_playlist: CreatePlaylist for the create_playlist method (for example), you can just pass in playlist_name: str. This logic would apply for the other classes you remove as well.

3. I suggest that it might be better if users are allowed to remove playlists just like they can remove songs. This would enhance the overall user experience because users will not have to go through the burden of having to remove every song from the playlist just to delete the playlist.

4. For the play_playlist method, there are many things going on and it seems confusing.

• I suggest breaking it down into smaller specific methods for better readability and debugging purposes.

5. I would consider more edge cases such as when the playlist_id: int is invalid when adding a song to a playlist for your future implementation.

songs.py
6. For the add_song method, since it takes in link: str as a parameter, it is prone to malicious/invalid links.

• Therefore, I suggest incorporating a validation mechanism to verify the link’s legitimacy and confirm its connection to the intended site.

7. I feel like the lower half code of the play_song method is redundant when it comes to error handling for checking if the user/song ID is valid or not. I do believe that it can be condensed.

8. For the parameters of the remove_song method, don’t you need a playlist_id so that users can choose which specific playlist to remove the song from? Or else from where is the song being removed from?

• If you haven’t thought about that yet, it would be a good idea to allow users to remove songs from the different playlists they have.

users.py
9. The attributes album: str, artist: str, link: str in the Platform class are never used.

• Are you planning to use it in your future implementation? If not, I would just remove it.

10. There is a MAJOR security concern when I create a user. The password created by the user can be seen in the request URL https://songstackify.onrender.com/users/create?password=random123

• I suggest taking a look again at how you are storing passwords in your create_user method.

11. I also suggest taking a look at built in libraries online for storing/handling passwords so that your service is more secure.

12. I suggest adding condition statements to handle errors for delete_user method (let’s say when someone inputs the wrong user_id: int or password: str intentionally or just by mistake).