AudioKit / Tonic

Swift library for music theory
https://www.audiokit.io/Tonic/
MIT License
154 stars 21 forks source link

Codable Support for all Types #21

Closed SebastianBoldt closed 9 months ago

SebastianBoldt commented 10 months ago

Description

Wouldn't it be nice if all the types are codable by default so we can easily persist them on disk, create json representations etc. ?

Proposed Solution

I guess there is not a lot to do to make this possible because most of the types already use basic foundation types under the hood.

Describe Alternatives You've Considered

Implementing on my own.

import Tonic

extension Tonic.Chord: Codable {
    private enum CodingKeys: String, CodingKey {
        case root
        case type
        case inversion
    }

    public init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        let root = try container.decode(NoteClass.self, forKey: .root)
        let type = try container.decode(ChordType.self, forKey: .type)
        let inversion = try container.decode(Int.self, forKey: .inversion)
        self.init(root, type: type, inversion: inversion)
    }

    public func encode(to encoder: Encoder) throws {
        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(root, forKey: .root)
        try container.encode(type, forKey: .type)
        try container.encode(inversion, forKey: .inversion)
    }
}

extension Tonic.NoteClass: Codable {
    private enum CodingKeys: String, CodingKey {
        case accidental
        case letter
    }

    public init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        let accidental = try container.decode(Accidental.self, forKey: .accidental)
        let letter = try container.decode(Letter.self, forKey: .letter)
        self.init(letter, accidental: accidental)
    }

    public func encode(to encoder: Encoder) throws {
        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(accidental, forKey: .accidental)
        try container.encode(letter, forKey: .letter)
    }
}

extension Tonic.Letter: Codable {}
extension Tonic.Accidental: Codable {}
extension Tonic.ChordType: Codable {}

Additional Context

Unfortunately conformance can not be synthesized automatically in an extra file so I have to implement manually as shown above.

Bildschirmfoto 2024-01-25 um 21 57 08
Matt54 commented 10 months ago

In my app "Chordable", I did make ChordType Codable for saving data and I'll add some notes based on my run in with the topic.

The problem I ran into: While ChordType does have an underlying primitive representation, it is not stable. The integer value is based on the order of the cases (majorTriad = 0, minorTriad = 1, etc....)

So, when a new case is added anywhere but the end (like we just did with adding suspendedSecondTriad) all of the cases after that case now have a new value (+1 from before). This breaks all previously saved data.

My Solution: Implement Codable in my own project and associate each ChordType with a stable string representation so that no matter what changes occur to the order of these cases in the framework, my saved data will be okay. Here's a gist of my codable conformance: https://gist.github.com/Matt54/7d41bebdbcf4b30d75de77cd45c0aed9

aure commented 10 months ago

Matt can you send us a PR with your codable support? Seems good.

SebastianBoldt commented 10 months ago

@aure Is it possible to change the raw type of the ChordType from Int to String? Alternatively, if we hard code the Values to the cases, it would also prevent new cases result in deserialization issues.

/// Major Triad: Major Third, Perfect Fifth
case majorTriad = 0

/// Minor Triad: Minor Third, Perfect Fifth
case minorTriad = 1

But I guess that using raw strings to describe the cases fits better than using a raw number.

SebastianBoldt commented 10 months ago

We have some other issues to address as well. One problem is with the Scale Type, that it conforms to OptionSet that results in RawRepresentable which is by default used in Codable. Unfortunately, this causes the description to disappear because we cannot retrieve it from the raw value.

public init(rawValue: Int) {
    self.rawValue = rawValue
    description = ""
}

It seems that something similar to this would be more appropriate.

extension Tonic.Scale: Codable {
    private enum CodingKeys: String, CodingKey {
        case intervals
        case description
    }

    public init(from decoder: Decoder) throws {
        let container = try decoder.container(keyedBy: CodingKeys.self)
        let intervals = try container.decode([Interval].self, forKey: .intervals)
        let description = try container.decode(String.self, forKey: .description)
        self = .init(intervals: intervals, description: description)
    }

    public func encode(to encoder: Encoder) throws {
        var container = encoder.container(keyedBy: CodingKeys.self)
        try container.encode(intervals, forKey: .intervals)
        try container.encode(description, forKey: .description)
    }
}

I think I will create proper codable support for all Types and open up a PR in a couple of days 😄

aure commented 10 months ago

Thanks @SebastianBoldt I went ahead and changed ChordType to String because it was easy, but please do make that PR with other more complex ones.

Matt54 commented 10 months ago

Nice, that is definitely the easiest fix! I wasn't sure if there was a reason why we wanted it to be an Int, such as some Objective-C support.

Another handy thing about using String for the raw value is that when looking at your encoded json you've got a human readable value.

aure commented 9 months ago

I assume this was taken care of by the pull request, we can reopen if needed.