Skip to content

Song should be immutable #328

@Abestanis

Description

@Abestanis

With the last update of equatable in #205 the EquatableMixin we use in Content has been marked as immutable (PR). This causes a linter warning on Song, because it contains non-final and therefore mutable members, most importantly the id.

Their reasoning for this change is sound, an objects hashcode should not change, otherwise it has weird behaviors in hash based collections, and I also think that it would simplify our code if our data models are immutable. (Sidenote: I think requiring the whole class to be immutable is a mistake of equatable. It should only apply to the props, since those are the only members contributing to hashCode.)

We have a rather convoluted system to allow duplicates in queues and playlists, which involves modifying the id and idMap. I propose that we replace that system and use a wrapper class instead that contains the unique id. We can then use that id where necessary, e.g. as the mediaId in the player. Something like this:

class QueueSong {
    final int uniqueId;
    final Song song;
}

We could also make it generic to be able to hold any content if necessary.

I would like to start working on this but want to make sure that you are ok with this approach @nt4f04uNd.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    Status

    Todo

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions