Skip to content

CodableCBOREncoder: save order for encoding Codable objects #117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Jnis
Copy link

@Jnis Jnis commented Apr 3, 2025

I must send CBOR with a specially ordered data. But current implementation shuffle they everytime.
This fix allow to save order for encoding Codable objects.

ex.:

struct MyData1: Codable {
    let a: Int
    let b: Int
    let c: Int
}

// custom keys and order example
struct MyData2: Codable {
    let a: Int
    let b: Int
    let c: Int
    
    enum CodingKeys: Int, CodingKey {
        case c = 3
        case b = 2
        case a = 1
    }
}

let myData1 = MyData1(a: 1, b: 2, c: 3)
let myData2 = MyData2(a: 1, b: 2, c: 3)
let encoder = CodableCBOREncoder()
let result1 = try! encoder.encode(myData1) // a3 61 61 01 61 62 02 61 63 03
let result2 = try! encoder.encode(myData2) // a3 03 03 02 02 01 01 

Copy link
Collaborator

@hamchapman hamchapman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR!

We won't want to merge this as-is because it's imposing quite a large cost on users who don't care about the ordering. Every time a new item is added to a keyed container it has to iterate over the entirety of the existing storage for the container to check if there's already an equivalent key in there, which isn't efficient.

For something like this to be merged I think a couple of things would need to change:

  1. the behaviour should be opt-in rather than the default
  2. the implementation should probably use something like an OrderedDictionary internally, or perhaps you'd just need a partial version of that and do something more minimal just using an OrderedSet

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants