Implementation of a shared music player

Hello Rustaceans,

As a part of my studies, I have written a small project which allows users to submit tracks to a music player via HTTP API. It also allows user to control the player.

The project is still in early stage and does not offer a lot of functionality, but I would like to hear your feedback on it. I do not want the project to be overengineered and keep it extensible.

Github Repository

I would really appreciate any help and advice!

Best regards,
Kyr

2 Likes

Interesting! A clone of the jukebox web app :grinning:. Will take a look later today and see if I can give you any feedback.

1 Like

Some thoughts, in no particular order:

  • In LocalStorage you are using two Mutexes:
#[derive(Default)]
pub struct LocalStorage {
    inner: Mutex<Vec<Track>>,
    current_index: Mutex<u64>
}

This can lead to a deadlock if two different threads are contending for the locks, and each one holds one of the locks already. I'm not sure if Tokio's Mutex has any prevention mechanism for deadlocks, but the recommendation is that you use a single mutex to access the whole LocalStorage struct.

  • I checked a few files, and saw that you use the if let syntax everywhere for matching on Option, Result, enums, etc. Personally I consider this not ideal because the if let syntax is is harder to parse than, let's say, a .map or a .map_err.

  • Try to run cargo fmt and cargo clippy on your project, it will make your code more idiomatic and easy to read.

  • In a few places I saw the use of the return keyword. For expression-based languages such as Rust, this is frown upon and considered bad practice. The suggested code-flow is that the last expression in a function is what will be returned from it, so you should structure your code in such a way that all the transformations happen on that very last expression.

  • I would move AsyncStorage to its own file, and limit the use of mod.rs files for the declaration and control of the visibility of modules, just like you did in /api/mods.rs.

In general I would say that your project is very clean and idiomatic! Nice job! :smiley:

2 Likes

Thank you for your feedback and detailed explanations!

I will refactor the LocalStorage and remove unnecessary Mutex. cargo fmt formatted my code, it definitely looks better now. Unnecessary returns will be removed and I will swap if let with .map and .map_err.

Thank again for your time and effort. I really appreciate your review.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.