[Swift] Does <modelName>API class need to be an inner class of <projectName>API ?

[hideya]

This is a discussion on request for improvement (not a bug).
For example for the Petstore demo, PetAPI class is generated as a inner class of PetstoreClientAPI:

extension PetstoreClientAPI {   
    public class PetAPI: APIBase {

Because of that, code to invoke a method of PetstoreClientAPI becomes a bit lengthy like the following:

        let newPet = Pet()
        ...
        PetstoreClientAPI.PetAPI.addPet(body: newPet).execute().then { response -> Void in

I'm wondering if this could just be PetAPI.addPet(body: newPet) without PetstoreClientAPI.?
Note that the model class Pet is not an inner class and can be accessed by just saying Pet. I don't see any strong reason for PetAPI being an inner class. What would you folks think?

[wing328]

Disclaimer: I'm no expert in Swift.

From what I read, Swift's extensions allows developers to:

• Add computed properties and computed type properties
• Define instance methods and type methods
• Provide new initializers
• Define subscripts
• Define and use new nested types
• Make an existing type conform to a protocol

I'm not sure if anyone is leveraging these features in the Swift SDK at the moment but personally I don't see drawback providing the flexibility at the moment (to meet the requirement of various use cases)

You do have a point that PetstoreClientAPI is a bit lengthy. Personally I also prefer something shorter. Did Apple have any recommended naming convention for extensions? (I couldn't find any...)

What about removing API from the extension? I believe PetstoreClient is also customizable via {{projectName}} (e.g. using Petstore instead of PetstoreClient).

[Edubits]

We had a short discussion this earlier as part of #901, where @tkqubo explained he intended it as namespace. In my personal opinion it's not needed to use it, but it shouldn't hurt either (except for the longer method calls). When you currently make it for example a Cocoapods package, there is also a kind of namespace as it will be a separate library.

In Swift 3 a Package Manager will be introduced (see https://swift.org/package-manager/), this will make it officially possible to have the same benefits without the need of the extra verboseness.

[hideya]

Thanks @wing328 and @Edubits for getting back to me! @wing328 yes those are the features of extension which realise enhancement of a class separately defined (in this case, public class CoffeeShopClientAPI defined in APIs.swift). That mechanism is used to introduce a namespace as @Edubits mentioned.

@Edubits I also think that we don't need this extra namespace since the generated code can be put in a framework/module which provides a namespace (thanks for the info about Swift 3 Package Manager).

If we really want to have a separate namespace, I think we should put model classes like Pet in the same namespace too and get rid of the API postfix.

(please let me include @bajtos for this discussion)

[hideya]

@wing328 @Edubits @tkqubo I don't want to rush the discussion, but I will have some spare time next week to work on this if necessary. So, I thought to ping you folks for your input.

My proposal is to remove the {{projectName}}API namespace. Although we can use typealias like typealias PetAPI = PetstoreClientAPI.PetAPI to mitigate the extra typing, I'd rather like to see cleaner code generation. What you folks think?

[wing328]

@hideya would it be hard to make it a CLI option (e.g. swiftUseExtension and default to false)?

That way developers have the option to generate Swift API client that's backward compatible.

[hideya]

@wing328 thanks for getting back to me! Could I ask you a clarification? Since PR #2244 is already breaking the backward compatibility, and since the current {{projectName}}API namespace looks halfway done to me (i.e. model classes are not in the namespace), I don't personally feel much value to keep supporting the backward compatibility of the {{projectName}}API namespace. What would you think?

[wing328]

My view is that just one non-backward compatible change will be included in the upcoming release does not mean we should include even more. Our goal is to minimize the impact to the developers if possible (ideally the upgrade should be seamless).

That's why I suggested we make a CLI option for this change and default to false, which means developers have an option to fallback if needed (that's not to say technically they cannot update their code to work with the Swift SDK generated by upcoming stable release of codegen)

If it's not easy to achieve that, then we've no choice but to consider a breaking change and have to rely on communication to notify the developers.

For this case, I'm ok if this is a breaking change. (I said the above just want to make sure you're aware or have considered the above approach)

[hideya]

@wing328 Thanks for the comment back! OK, all sounds reasonable. (I do value backward compatibility but mentioned like above since the exact lines that become broken by #2244 will be broken by this change too. So, I thought that these two braking changes can be treated as single one when to explain the developers)

I'll check out how easy/hard it would be and get back to you!

[hideya]

@wing328 I figured that it is quite straight forward to keep supporting the {{projectName}}API namespace together with the proposed simplification without messing APIs.mustache.

So, I will work on the CLI option stuff too following your recommendation. I'm thinking to introduce swiftUseApiNamespace flag, and if a user would like to use the namespace, the user adds -D swiftUseApiNamespace=true to the command line. Does my understanding correct?

[wing328]

@hideya yup, that's how I understand it.

[hideya]

@wing328 I finished the changes including the CLI option. I made sure updated unit tests (w/o PetstoreClientAPI.) works fine.

Now, could I ask your opinion? Would it be ok for me to merge the changes into PR #2244? I didn't need to touch many lines for this one. I thought it might be easier to keep track of those two related breaking changes if both are done by one PR, with providing single "Before/After" description. Of course, I'm fine with filing a separate PR after #2244 is merged. Which would you like me to do?

[wing328]

Fine with me to merge the changes into #2244.

[hideya]

Thanks! I'll work on it then!