Skip to content
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

[Swift4] Change model object from class to struct #6941

Closed
d-date opened this issue Nov 13, 2017 · 20 comments · Fixed by #9932
Closed

[Swift4] Change model object from class to struct #6941

d-date opened this issue Nov 13, 2017 · 20 comments · Fixed by #9932

Comments

@d-date
Copy link
Contributor

d-date commented Nov 13, 2017

Description

I wonder why model templates of swift 4 is type of open class, instead of public struct.

Swagger-codegen version

2.3.0

Swagger declaration file content or url
Command line used for generation
Steps to reproduce
Related issues/PRs
Suggest a fix/enhancement

In Swift 4, all of generated model class is adopted for Swift.Codable, which is protocol so that we can use struct instead of class, and it's more comfortable for Modal object.

So, try to change as below.

  • Before
open class Example : Swift.Codable {
  
  public var id: Int32

  public var name: String

}
  • After
public struct Example : Swift.Codable {

  public var id: Int32

  public var name: String

}

(I think they should be let as get is defined in yaml.)

@wing328
Copy link
Contributor

wing328 commented Nov 13, 2017

@d-date thanks for the suggestion.

Here are similar discussions/suggestions before:

#5761
#4126

@wing328
Copy link
Contributor

wing328 commented Nov 13, 2017

cc @jgavris @ehyche @Edubits @jaz-ah

@jgavris
Copy link
Contributor

jgavris commented Nov 13, 2017

I'm big +1 on this.

@ehyche
Copy link
Contributor

ehyche commented Nov 13, 2017

I like this, and considered doing it for my initial implementation.

However, when we have allOf/discriminator in the swagger schema, then we need to inherit properties from other models. The easiest way to do this is to use inheritance, which we can't do with structs. So would you rather:

  • Have structs when we don't have a parent, and have classes when we do? OR
  • Have classes everywhere?

Alternatively, I guess we could probably not worry about shared code, and just put parent properties directly into the child struct and then duplicate them in the parent struct as well.

Thoughts, @jgavris @Edubits @jaz-ah ?

@jaz-ah
Copy link
Contributor

jaz-ah commented Nov 13, 2017

i'm +1 on the concept - @ehyche i'd rather be consistent and either always have structs and duplicate properties or always have classes at the end of the day.

@siftikha
Copy link

Is there a compelling reason not to offer both and let the user choose? It might also be nice to offer it as a protocol.

@ehyche
Copy link
Contributor

ehyche commented Jan 2, 2018

I have a PR for this - just waiting for a dependent PR to be merged, and then I'll put it up.

ehyche pushed a commit to ehyche/swagger-codegen that referenced this issue Jan 8, 2018
This fixes issue swagger-api#6941.

In this change, we make our Swift4 generated model objects struct instead of class. However, in order to do this, we needed to handle the following edge cases:

* Inheritance and polymorphism (allOf)
  * With classes, we use inheritance. So therefore, the parent properties are ONLY on the parent generated class, and the model object which derives from the parent class picks up those properties through inheritance.
  * However, structs do not support inheritance. So we simply duplicate the parent allOf properties in the child struct.
* We have to handle the case where the property name on the struct may be different than the property name in the JSON. By default, the Codable protocol assumes that the JSON property name is the same as the struct property name. If they need to be different, then we generate a CodingKeys string enum, which contains the mapping between struct property name and JSON property name.
* additionalProperties. We cannot use the default Codable implementation for the additionalProperties, since it will look for an actual dictionary called "additionalProperties" in the JSON. Therefore, for model objects which have additionalProperties, we must generate our own implementation for the Decodable and Encodable protocols.

I have run ./bin/swift4-all.sh and ./bin/swift4-test.sh to re-generate all of the sources, and I have verified that the generated code in samples/clients/test/swift4/default builds and the unit tests pass.
wing328 pushed a commit that referenced this issue Jan 25, 2018
* Split up model template into partials

* Change models from class to struct.

This fixes issue #6941.

In this change, we make our Swift4 generated model objects struct instead of class. However, in order to do this, we needed to handle the following edge cases:

* Inheritance and polymorphism (allOf)
  * With classes, we use inheritance. So therefore, the parent properties are ONLY on the parent generated class, and the model object which derives from the parent class picks up those properties through inheritance.
  * However, structs do not support inheritance. So we simply duplicate the parent allOf properties in the child struct.
* We have to handle the case where the property name on the struct may be different than the property name in the JSON. By default, the Codable protocol assumes that the JSON property name is the same as the struct property name. If they need to be different, then we generate a CodingKeys string enum, which contains the mapping between struct property name and JSON property name.
* additionalProperties. We cannot use the default Codable implementation for the additionalProperties, since it will look for an actual dictionary called "additionalProperties" in the JSON. Therefore, for model objects which have additionalProperties, we must generate our own implementation for the Decodable and Encodable protocols.

I have run ./bin/swift4-all.sh and ./bin/swift4-test.sh to re-generate all of the sources, and I have verified that the generated code in samples/clients/test/swift4/default builds and the unit tests pass.

* Update VERSION in .swagger-codegen

* Update generated code for swift4-test schema
@ejithon
Copy link
Contributor

ejithon commented Feb 9, 2018

Initializers are no longer see from other package. Could you add initializer again? I am usually using the generated code as a CocoaTouch Framework to reuse this via Carthage.

@Zexbe
Copy link

Zexbe commented Mar 6, 2018

This needs to do something special for recursion.

public struct Node : Swift.Codable {
  public var id: Int32
  public var name: String
  public var next: Node?
}

@Crysis21
Copy link

Crysis21 commented Mar 7, 2018

I've just updated my codegen to generate swift4 code. How do I enable class generation instead of struct? I'll have a great deal of refactoring, not to mention that I need the object references, as my code passes the same object to different views and I don't need multiple copies.

@ehyche
Copy link
Contributor

ehyche commented Mar 7, 2018

There is not an option to switch between class and struct. The swift4 module always generates structs now.

@Crysis21
Copy link

Crysis21 commented Mar 7, 2018

thank you, I thought so after seeing the moustache files, but I wanted to be certain. I switch would've been handy for people who want to port complex projects, or if they just want to take advantage of classes.

@eoinnorris
Copy link

Classes are essential for Objective C interoperability.

@jgavris
Copy link
Contributor

jgavris commented Jun 27, 2018

@eoinnorris I agree, but isn't it a bit strange to expose Swift classes to Objective-C? I often do the opposite. Maybe that's just a side effect of having a nearly pure Swift application. The Objective-C bits are usually just there to bridge C++ code into Swift.

@eoinnorris
Copy link

eoinnorris commented Jun 28, 2018

@jgavris the particular use case I have is a swift framework, which may be consumed by an Objective C app. We are designing the SDK to interact with our services, and have written it for swift for future proofing. However we can't really assume that only swift apps will use it. Probably this will become even more common in swift 5. In fact in our demo app is a react app, and the react bridge is in Objective C only.

Besides classes, all public instances need an @objc.

I need to do this anyway, so I could take a look at whether its worth doing in CodeGen as an option, or write a separate script myself.

@aduval
Copy link

aduval commented Aug 3, 2018

Classes are useful when the model uses inheritance and polymorphism. We can't take advantage of these object-oriented basic features anymore. Like many others, I will need to maintain my own templates (and it's a pain). Classes should still be supported (as a first-class alternative).

@alokc83
Copy link

alokc83 commented Apr 24, 2019

@eoinnorris I agree, but isn't it a bit strange to expose Swift classes to Objective-C? I often do the opposite. Maybe that's just a side effect of having a nearly pure Swift application. The Objective-C bits are usually just there to bridge C++ code into Swift.

I do agree that structs are much more powerful in swift. but neither of both have capabilities to replace one another. Not sure who decided not to support both.

@d-date
Copy link
Contributor Author

d-date commented Apr 24, 2019

We've already supported on our fork. This is main stream about Swift codegen.

https://github.com/OpenAPITools/openapi-generator

@d-date d-date closed this as completed Apr 24, 2019
@HugoMario HugoMario reopened this Dec 8, 2019
@Pe-te
Copy link

Pe-te commented Nov 17, 2020

@HugoMario You merged the new additionalProperty "useModelClasses" into master (Dec 2019), but it seems still missing in V3.0.23 (Nov 2020)?

@HugoMario
Copy link
Contributor

thanks @Pe-te for letting me know. I just added and merged this PR to port changes

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

Successfully merging a pull request may close this issue.