-
Notifications
You must be signed in to change notification settings - Fork 3
Refactor Hit struct to be an enum? #10
Comments
I don't use enums because I have no experience with them, the examples of the enum representation of serde seems disheartening, if you have an example of this I will check. Genius API supports many types of searches, but they only offer developers a small music search, in a no longer future I will support these types. |
Without confirming (I am away from a computer at present), a possible representation could look like: #[derive(Deserialize, Debug)]
#[serde(tag = "type", content = "result")]
#[non_exhaustive]
pub enum Hit {
#[serde(alias = "song")]
Song(Song)
} This uses serde's support for adjacent tagging to handle possible outcomes. This sample code may not be ideal, as it does remove support for the index value, but by my recollection, that information isn't really something consumers of the crate would use, since they would presumably get whatever results they need in the Vec of Hits. If the index is used for pagination, that's a crate implementation detail. |
Hmm, the way you did it would be possible: #[derive(Deserialize, Debug)]
#[serde(tag = "type", content = "result")]
#[serde(tag = "type", content = "album")]
#[non_exhaustive]
pub enum Hit {
#[serde(alias = "song")]
Song(Song)
#[serde(alias = "album")]
Album(Album)
} ? |
I reviewed the API responses for certain types of search and I think that even, so I will have to write another structure to treat some cases. |
Love this crate. API wrappers are the unsung heroes of any application ecosystem, and I'm glad to see any improvements to Rust's application-level situation. Having said that, I'm curious if this library can map to some common Rust idioms.
In this case, as of version 0.4.0, this library still uses a struct to represent a Hit, or search result from the Genius API.
genius-rs/src/search.rs
Lines 5 to 11 in 283c082
Can this be changed?
serde
allows the deserialization of enums, and if there are even types other than "song" that would appear in the type field, an enum could be used to represent them. It would certainly be much easier to use than having to read all the public fields of each type.I couldn't find a complete list of "type"s that Genius search hits can have, but since they seem to only currently support a song type, it is likely they may simply have future proofed, intending to update their database later on to distinguish between music videos and songs, which they currently do not do. For the time being, a single-variant,
#[non-exhaustive]
enum could represent this perfectly.The text was updated successfully, but these errors were encountered: