spotify-dart icon indicating copy to clipboard operation
spotify-dart copied to clipboard

Organize model classes

Open hayribakici opened this issue 1 year ago • 5 comments

I figured out, that the model classes are very messy with many duplicates, which makes this library quite error prone.

In order to give more clarity, I am suggesting to model the library as shown in this diagram:

The main thing here are the two classes SpotifyContentBase and SpotifyContent, which all of the content classes (Tracks, Playlist, Album, Episodes, Show, Artist, the last one is really debatable). The idea behind this is that every class that has the attributes id, type, href and uri is considered a SpotifyContent.

Furthermore, I am suggesting to add some "model building blocks" in the form of mixins where model classes can just implement them and include some model properties. Furthermore, this could be also useful, if the @JsonKey needs a fromJson function, so that code is not copied around. This could look like as follows:

class Foo with BuildingBlock {
  
  @JsonKey(name: 'some_property')
  int someProperty;
}

mixin BuildingBlock {

  static String _extractAnotherProperty(dynamic json) => json['attr'];
  
  String? _anotherProperty;

  @JsonKey(name: 'another_property', fromJson: _extractAnotherProperty)
  String? get anotherProperty => _anotherProperty;
}

In order to properly inherit the attribute, it is important to wrap it in a property. Concretely, mixins such as Copyright, ExternalUrls, AvailableMarkets and Popularity (as discussed in #138) could be one of those (to name a few) building blocks.

What do you think?

EDIT:

Regarding this bit:

Having an inheritance hierarchy does come with some perks. Prior, classes like TrackSaved have their own respective member: track or playlist (in PlaylistsFeatured). These members are now refactored out to the Content class with the generic member T? content. So in order to force users to override the T? content member and annotate it with the appropriate JsonKey, the classes need to realize the SavedContent<T> or FeaturedContent<T>:

abstract class _Content<T> {
  T? content;
}

abstract class _FeaturedContent<T> implements _Content<T> {
  String? _message;

  @JsonKey()
  String? get message => _message;
}

class CategoryPlaylist with _FeaturedContent<PlaylistSimple> { // also contains message
  @override
  @JsonKey(name: 'playlists')
  PlaylistSimple? content;
}

hayribakici avatar Sep 22 '23 08:09 hayribakici

Thanks for the writeup and input - will take an indepth look at this hopefully this weekend!

rinukkusu avatar Sep 27 '23 19:09 rinukkusu

Thank you!

hayribakici avatar Sep 28 '23 19:09 hayribakici

I completely agree - it's a little messy to work with and reason about. The current implementation is a naive 1:1 mapping of the Spotify API models from the documentation with few abstractions. This is also from a time, where mixins etc were not a thing. 😆

Your mockup of the the class design seems pretty reasonable - you took the time to go through every model and checked what field can be extracted, very nice!

Regarding the building blocks section: this is what you mean with the mixin "type" in your diagram, right?

rinukkusu avatar Nov 28 '23 20:11 rinukkusu

Yes, exactly. The mixins diagramss are labeled as such.

hayribakici avatar Nov 28 '23 20:11 hayribakici

I tried the above approach with the mixins on my local branch. Unfortunately, the concept does not work. jsonserializer does not generate the necessary code for the attributes from the mixin. Meaning, a class

class Album with Popularity {
  String? name;
}

mixin Popularity {
  int? popularity;
}

wont generate the code for the attribute popularity and fill it with a value accordingly. Just wanted to share my insights.

hayribakici avatar Jul 23 '24 11:07 hayribakici