[PR #305] [MERGED] Meris/refactor playable playcontext #388

New issue

Closed

opened 2026-02-27 20:24:31 +03:00 by kerem · 0 comments

kerem commented

2026-02-27 20:24:31 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/305
Author: @merisbahti
Created: 3/10/2022
Status: ✅ Merged
Merged: 7/13/2022
Merged by: @marioortizmanero

Base: master ← Head: meris/refactor-playable-playcontext

📝 Commits (10+)

5e90864 fix: start refactor
a6c0654 fix some more refactor
62ba63f fix: it compiles
e43c253 More performant version done!
cbc1177 Almost there
0f6d439 introduce as_borrowed() for id types
e07a0ed fix tests
b25790a Rename as_borrowed to as_ref
1799615 Fix part of the docs
3f787bb Considerably neater implementation

📊 Changes

30 files changed (+680 additions, -532 deletions)

View changed files

📝 CHANGELOG.md (+3 -0)
📝 examples/auth_code.rs (+1 -1)
📝 examples/auth_code_pkce.rs (+2 -2)
📝 examples/client_creds.rs (+2 -2)
📝 examples/tasks.rs (+1 -1)
📝 examples/ureq/device.rs (+1 -1)
📝 examples/ureq/me.rs (+1 -1)
📝 examples/ureq/search.rs (+12 -12)
📝 examples/ureq/seek_track.rs (+2 -2)
📝 examples/ureq/threading.rs (+1 -1)
📝 examples/webapp/Cargo.toml (+7 -4)
📝 examples/webapp/src/main.rs (+10 -9)
📝 examples/with_auto_reauth.rs (+8 -7)
📝 examples/with_refresh_token.rs (+7 -6)
📝 rspotify-model/Cargo.toml (+1 -1)
📝 rspotify-model/src/album.rs (+2 -2)
📝 rspotify-model/src/artist.rs (+2 -2)
📝 rspotify-model/src/audio.rs (+1 -1)
📝 rspotify-model/src/idtypes.rs (+388 -295)
📝 rspotify-model/src/lib.rs (+4 -3)

...and 10 more files

📄 Description

Description

Basically change the two traits PlayContextId and PlayableId, from this:

pub trait PlayableId: Id {}
pub trait PlayContextId: Id {}

impl PlayContextId for ArtistId {}
impl PlayContextId for AlbumId {}
impl PlayContextId for PlaylistId {}
impl PlayContextId for ShowId {}
impl PlayableId for TrackId {}
impl PlayableId for EpisodeId {}

To use enums, like this:

pub enum PlayContext {
    Artist(ArtistId),
    Album(AlbumId),
    Playlist(PlaylistId),
    Show(ShowId),
}
pub enum Playable {
    Track(TrackId),
    Episode(EpisodeId),
}

Motivation and Context

So that we don't have to use dynamic dispatch, and don't have to see code like this:

let sliced = track_ids
    .into_iter()
    .map(|track_id| TrackId::from_id(&track_id).unwrap())
    .collect::<Vec<_>>();
let playable = sliced
    .iter()
    .map(|x| x as &dyn PlayableId)
    .collect::<Vec<_>>();

Dependencies

None

Type of change

Please delete options that are not relevant.

Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How has this been tested?

Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce.

Please also list any relevant details for your test configuration

Test A: xxx
Test B: xxx

Is this change properly documented?

Please make sure you've properly documented the changes you're making.

Don't forget to add an entry to the CHANGELOG if necessary (new features, breaking changes, relevant internal improvements).

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/ramsayleung/rspotify/pull/305 **Author:** [@merisbahti](https://github.com/merisbahti) **Created:** 3/10/2022 **Status:** ✅ Merged **Merged:** 7/13/2022 **Merged by:** [@marioortizmanero](https://github.com/marioortizmanero) **Base:** `master` ← **Head:** `meris/refactor-playable-playcontext` --- ### 📝 Commits (10+) - [`5e90864`](https://github.com/ramsayleung/rspotify/commit/5e9086422c9a2d6d4a822f2daf9056511a374670) fix: start refactor - [`a6c0654`](https://github.com/ramsayleung/rspotify/commit/a6c0654b996482cfac6fad239756aa6aded6fbc3) fix some more refactor - [`62ba63f`](https://github.com/ramsayleung/rspotify/commit/62ba63f61c4abf50381887677aff96c6bc43435a) fix: it compiles - [`e43c253`](https://github.com/ramsayleung/rspotify/commit/e43c253c7cda2ef376e7fae2f16af8989b63bf23) More performant version done! - [`cbc1177`](https://github.com/ramsayleung/rspotify/commit/cbc1177ed9039e49d5cf79489113c76d4ead1d21) Almost there - [`0f6d439`](https://github.com/ramsayleung/rspotify/commit/0f6d43963f063946c081b0bfaf2285518809e35f) introduce as_borrowed() for id types - [`e07a0ed`](https://github.com/ramsayleung/rspotify/commit/e07a0ed107915c1033e9178b162656ad21cf52b6) fix tests - [`b25790a`](https://github.com/ramsayleung/rspotify/commit/b25790aedc544025f3f3b3733602fd72b4dc05be) Rename as_borrowed to as_ref - [`1799615`](https://github.com/ramsayleung/rspotify/commit/17996150a978021914abf3c02af9427b9cd561d7) Fix part of the docs - [`3f787bb`](https://github.com/ramsayleung/rspotify/commit/3f787bb663c7810288370a2e3929c0938023c0c0) Considerably neater implementation ### 📊 Changes **30 files changed** (+680 additions, -532 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+3 -0) 📝 `examples/auth_code.rs` (+1 -1) 📝 `examples/auth_code_pkce.rs` (+2 -2) 📝 `examples/client_creds.rs` (+2 -2) 📝 `examples/tasks.rs` (+1 -1) 📝 `examples/ureq/device.rs` (+1 -1) 📝 `examples/ureq/me.rs` (+1 -1) 📝 `examples/ureq/search.rs` (+12 -12) 📝 `examples/ureq/seek_track.rs` (+2 -2) 📝 `examples/ureq/threading.rs` (+1 -1) 📝 `examples/webapp/Cargo.toml` (+7 -4) 📝 `examples/webapp/src/main.rs` (+10 -9) 📝 `examples/with_auto_reauth.rs` (+8 -7) 📝 `examples/with_refresh_token.rs` (+7 -6) 📝 `rspotify-model/Cargo.toml` (+1 -1) 📝 `rspotify-model/src/album.rs` (+2 -2) 📝 `rspotify-model/src/artist.rs` (+2 -2) 📝 `rspotify-model/src/audio.rs` (+1 -1) 📝 `rspotify-model/src/idtypes.rs` (+388 -295) 📝 `rspotify-model/src/lib.rs` (+4 -3) _...and 10 more files_ </details> ### 📄 Description ## Description Basically change the two traits `PlayContextId` and `PlayableId`, from this: ```rust pub trait PlayableId: Id {} pub trait PlayContextId: Id {} impl PlayContextId for ArtistId {} impl PlayContextId for AlbumId {} impl PlayContextId for PlaylistId {} impl PlayContextId for ShowId {} impl PlayableId for TrackId {} impl PlayableId for EpisodeId {} ``` To use enums, like this: ```rust pub enum PlayContext { Artist(ArtistId), Album(AlbumId), Playlist(PlaylistId), Show(ShowId), } pub enum Playable { Track(TrackId), Episode(EpisodeId), } ``` ## Motivation and Context So that we don't have to use dynamic dispatch, and don't have to see code like this: ```rust let sliced = track_ids .into_iter() .map(|track_id| TrackId::from_id(&track_id).unwrap()) .collect::<Vec<_>>(); let playable = sliced .iter() .map(|x| x as &dyn PlayableId) .collect::<Vec<_>>(); ``` ## Dependencies None ## Type of change Please delete options that are not relevant. - [x] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [x] This change requires a documentation update ## How has this been tested? Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration - [ ] Test A: xxx - [ ] Test B: xxx ## Is this change properly documented? Please make sure you've properly documented the changes you're making. Don't forget to add an entry to the CHANGELOG if necessary (new features, breaking changes, relevant internal improvements). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>