[PR #242] [CLOSED] Fix IDs v2 #336

New issue

Closed

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

kerem commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/ramsayleung/rspotify/pull/242
Author: @marioortizmanero
Created: 8/24/2021
Status: ❌ Closed

Base: fix-ids ← Head: fix-ids-2

📝 Commits (2)

5d459f4 First draft with newtype
ea435db The compiler hates me

📊 Changes

8 files changed (+177 additions, -236 deletions)

View changed files

📝 rspotify-model/src/context.rs (+5 -5)
📝 rspotify-model/src/enums/types.rs (+0 -1)
📝 rspotify-model/src/idtypes.rs (+151 -201)
📝 rspotify-model/src/lib.rs (+9 -18)
📝 rspotify-model/src/offset.rs (+5 -5)
📝 rspotify-model/src/playing.rs (+2 -2)
📝 rspotify-model/src/recommend.rs (+4 -4)
📝 src/lib.rs (+1 -0)

📄 Description

Description

This attempt tries to implement what i described in #203:

A different approach would be to scratch Id<T> and just use ArtistId and similars directly. What I mean is that we wouldn't have a single type for IDs called Id<T> that implements its logic. We could make ArtistId and similars, and implement the trait the same way clients work. The main component would be a trait that implements all the logic, say Id, and a struct for each type (ArtistId, etc) that implements the trait. With this one could be generic over Id directly, have Box<dyn Id> for runtime usage, and Vec<Box>` for multiple values, which is exactly what we need.

The main problems are:

We can't use type inference to our benefit; Id::from_uri can't work with this solution; ArtistId::from_uri would have to be used explicitly.

We need the trait Id always available when using IDs; we'd have to add it to the prelude. Considering the user needs to import the prelude to use the clients anyway I don't think it's a big deal

So this solution is basically the same as how clients work. We have a big trait that does everything, and we let Rust "clone" the code for each of our implementations. It's also basically the newtype pattern, but with a common trait for all the newtypes.

_{🔄 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/242 **Author:** [@marioortizmanero](https://github.com/marioortizmanero) **Created:** 8/24/2021 **Status:** ❌ Closed **Base:** `fix-ids` ← **Head:** `fix-ids-2` --- ### 📝 Commits (2) - [`5d459f4`](https://github.com/ramsayleung/rspotify/commit/5d459f47b543e354d1fdb0911b7654749db94fc6) First draft with newtype - [`ea435db`](https://github.com/ramsayleung/rspotify/commit/ea435db5e9d13bb1c5497842b3b56c9a9ce719d6) The compiler hates me ### 📊 Changes **8 files changed** (+177 additions, -236 deletions) <details> <summary>View changed files</summary> 📝 `rspotify-model/src/context.rs` (+5 -5) 📝 `rspotify-model/src/enums/types.rs` (+0 -1) 📝 `rspotify-model/src/idtypes.rs` (+151 -201) 📝 `rspotify-model/src/lib.rs` (+9 -18) 📝 `rspotify-model/src/offset.rs` (+5 -5) 📝 `rspotify-model/src/playing.rs` (+2 -2) 📝 `rspotify-model/src/recommend.rs` (+4 -4) 📝 `src/lib.rs` (+1 -0) </details> ### 📄 Description ## Description This attempt tries to implement what i described in #203: > A different approach would be to scratch `Id<T>` and just use `ArtistId` and similars directly. What I mean is that we wouldn't have a single type for IDs called `Id<T>` that implements its logic. We could make `ArtistId` and similars, and implement the trait the same way clients work. The main component would be a trait that implements all the logic, say `Id`, and a struct for each type (`ArtistId`, etc) that implements the trait. With this one could be generic over `Id` directly, have `Box<dyn Id> for runtime usage, and `Vec<Box<dyn Id>>` for multiple values, which is exactly what we need. > > The main problems are: > > 1. We can't use type inference to our benefit; `Id::from_uri` can't work with this solution; `ArtistId::from_uri` would have to be used explicitly. > 2. We need the trait `Id` always available when using IDs; we'd have to add it to the prelude. Considering the user needs to import the prelude to use the clients anyway I don't think it's a big deal > > So this solution is basically the same as how clients work. We have a big trait that does everything, and we let Rust "clone" the code for each of our implementations. It's also basically the newtype pattern, but with a common trait for all the newtypes. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>