yonaskolb / SwagGen

OpenAPI/Swagger 3.0 Parser and Swift code generator
MIT License
626 stars 146 forks source link

`public var success:SuccessType?` - why optional? #183

Open haemi opened 5 years ago

haemi commented 5 years ago

why is the success-Type be an optional?

let request = API.GetXY.Request(...)
currentRequest = apiClient.makeRequest(request, behaviours: [], completionQueue: DispatchQueue.main) { result in
    switch result.result {
    case let .success(value):
        let xy = value.success?.data ?? []
        completion(.success(xy))
    case let .failure(error):
        if !error.isCancelled {
            completion(.failure(error))
        }
    }
}

I don't understand when value.success can be nil - I would expect this to always be set and never be nil, so an optional can be avoided IMHO. Am I missing something?

yonaskolb commented 5 years ago

Having a 200 range response in swagger is optional. You can edit the template if your api always has a success response though. This also lets you build a convenience function that fails on non success

yonaskolb commented 5 years ago

Thinking about this, an improvement could be to see if all the possible responses in a spec have 200 range responses and then making this non optional in the protocol

yonaskolb commented 5 years ago

Sorry, I actually missunderstood, the reason this is optional is that the endpoint may fail so the success property is optional. The initial result is if the endpoint returned anything at all, then you check why the response actually was. This can be abstracted in a wrapper function to just return a result with a generic failure and the success response. This will mean you won’t have typed errors though. This issue tracks adding such a simplified handler https://github.com/yonaskolb/SwagGen/issues/91

haemi commented 5 years ago

Thanks for the quick response!

Am I getting you right that it's nil if the request is successful, the response has a successful status code but the content is not in a format the API expects? Or what has to happen that the success-property is nil?

And: do you have plans to change this? Is that what #91 is for?