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

Full init constructor for provided types #91

Closed
yodiz opened this issue Nov 17, 2017 · 7 comments
Closed

Full init constructor for provided types #91

yodiz opened this issue Nov 17, 2017 · 7 comments
Milestone

Comments

@yodiz
Copy link

yodiz commented Nov 17, 2017

Description

As it is now Swaggerprovider generates mutable classes for objects. Would it be possible to generate immutable F# records?

Options types for non required fields would also be nice :)

If F# Records are not possible, is it possible to have a default constructor on the generated type that sets all properties of the object? This would minimize risk of not setting an property by accident. And also if the underlying swagger-specification change, and a property is added on the object: You would get a compile-error that you need to set the property.

@sergey-tihon
Copy link
Member

As it is now Swaggerprovider generates mutable classes for objects. Would it be possible to generate immutable F# records?

According to my knowledge - no, TP SDK does not allow to emit record types fsharp/fslang-suggestions#154 - you may vote for this feature if you like to see it in some future.

Options types for non required fields would also be nice :)

SwaggerProvider already provides Option<T> when T is not nullable - see here But when the T is nullable it is an ambiguous question what to provide. IMHO it is not good idea to generate object model that allow to have Some(null) values.

If F# Records are not possible, is it possible to have a default constructor on the generated type that sets all properties of the object? This would minimize risk of not setting an property by accident. And also if the underlying swagger-specification change, and a property is added on the object: You would get a compile-error that you need to set the property.

I understand you intend, but we still need default parameter-less constructor and property setters to allow serializer (JSON.NET) to deserialize server response to provided model classes. So we probably cannot avoid the risk of accident property setting.

The only thing we can do is to add one more constructor with enormous number of parameters that initialize all properties.

@yodiz
Copy link
Author

yodiz commented Nov 21, 2017

Having one empty constructor, and another one that set all the properties of the object would be very helpfull. Any idea when and if you will get around to it? I could give it a try, but i dont have much experience on the coding-side of typeproviders.

I would prefer having fields marked with required=false generated as an Option<_>. I see your point, but now you get no distinction if fields are required or not. How about a setting for SwaggerProvider<NonRequiredFieldsAsOptions=true> , or something of the sort :)

@sergey-tihon
Copy link
Member

sergey-tihon commented Nov 22, 2017

You can try it using v0.10.0-alpha1 and help me test other changes ;) - initial implementation is here 8eb0d88

2017-11-22_0853

@sergey-tihon
Copy link
Member

sergey-tihon commented Nov 22, 2017

if the property type is a number then you will see difference in types ...

type=number + required=true -> int
type=number + required=false -> Option<int>

but when the property is string then it will be string for all both options because null is the correct value for the string.

What will be the benefit of generating Option<string> properties?

@yodiz
Copy link
Author

yodiz commented Nov 22, 2017

Oh great, i'll test it. Thanks and well done :)

About the Option<_>:
I just prefer that the fields optionality is described in a "typed manner".

Let's assume you have an Object Pet that has a Required PrimaryCategory of the type PetCategory and another field SecondaryCategory that is optional. With an option type for reference-objects aswell that are optional i just think it gets more intuitive.

No option type:

{
  PetId : int
  PrimaryCatergory : PetCategory
  SecondayCategory : PetCategory // <-- is it required or not?
}

With option type:

{
  PetId : int
  PrimaryCatergory : PetCategory
  SecondayCategory : PetCategory option // <-- Optionality explained through type-system
}

I agree that its kind of dumb that you can provide Some(null). And it's just a matter of preference. For my usage i prefer the option type, but i get that you might not want that.

Thanks for the constructor-thingy again, And for all the type providers :)

@baronfel
Copy link
Contributor

@sergey-tihon perhaps for the case when the user has requested option types, we can do an Option.ofObj on the deserialized response, to eliminate Some(null) cases and keep the model as pure as possible?

@sergey-tihon sergey-tihon changed the title Is it possible to have SwaggerProvider generate F# records instead of mutable classes? Full init constructor for provided types Nov 23, 2017
@sergey-tihon
Copy link
Member

@baronfel yes, probably we should try it. I did open new issue to track this request #93

I close this one because full-init ctor is released in v0.10.0-alpha1 package.

@sergey-tihon sergey-tihon added this to the v1.0 milestone Mar 2, 2019
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

No branches or pull requests

3 participants