steamclock / netable

A Swift library for encapsulating network APIs using Codable in a type-oriented way.
MIT License
99 stars 3 forks source link

Change `jsonKeyEncodingStrategy` and `jsonKeyDecodingStrategy` to non-optional with default values #108

Open brendanlensink opened 1 year ago

brendanlensink commented 1 year ago

As they currently exist, you can create a request like

struct UserRequest: Request {
    // ...

    var jsonKeyDecodingStrategy: JSONDecoder.KeyDecodingStrategy { return .convertFromSnakeCase }
}

that looks correct but won't work, since it should be JSONDecoder.KeyDecodingStrategy?.

To fix this, and make it more difficult to mess up, I think we should switch both encoding strategy params to non-optional and just provide a default setting, like we do with smartUnwrapKey

amyoulton commented 1 year ago

Findings:

Currently we have two places that we can set encoding/decoding, the request itself or the netable implementation configurations. Currently, we're checking if there is a strategy within the individual request existing and if it doesn't exist, it defaults to the config one, which is set to use default keys.

Since we removed the optional, this would never happen, which means that config option is rendered useless.

We need to think on how we want to solve this issue as to not affect the current structure of Config being the base decoding/encoding strategy which is overridden by a requests if it exists! My gut tells me that we are going to want to keep it optional because this structure currently makes a lot of sense, but then I'm unsure how to handle that error we encountered.