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

DataClassContainsFunctions - allow conversion function #434

Closed
vanniktech opened this issue Sep 25, 2017 · 10 comments
Closed

DataClassContainsFunctions - allow conversion function #434

vanniktech opened this issue Sep 25, 2017 · 10 comments

Comments

@vanniktech
Copy link
Contributor

What do you think about an option that allows conversion functions that take the Data class and basically map it to a different type. For instance:

data class MySomething(val prop1: String, ...) {
  fun toSomething() = Something(prop1)
}
@schalkms
Copy link
Member

Is this related to the UseDataClass rule?
Does that mean that a normal class which uses conversion functions should also be a data class candidate?

@vanniktech
Copy link
Contributor Author

Hmm it might yeah. I'd say that conversion functions are defined with a prefix of to or as and then no arguments.

@arturbosch
Copy link
Member

Actually I also often use conversion functions for data classes but outside the class and in the same file. I would also argue that this should be the preferred way.

@tagantroy
Copy link
Contributor

@vanniktech @arturbosch Hello, should I add this feature to existing rules by default ( DataClassContainsFunctions, UseDataClass) or add additional configuration?

@arturbosch
Copy link
Member

@tagantroy I don't know. I still think such functions should be put at top level (extension functions). If you guys ( @vanniktech @schalkms ) give the issue a thumbs up, then why not @tagantroy :)

@schalkms
Copy link
Member

I agree with @arturbosch
I wouldn't add conversion functions to data classes. However, I also think an optional configuration is ok if it's set to false per default.

@arturbosch
Copy link
Member

@tagantroy here you have your go :) thanks for contributing!

@tagantroy
Copy link
Contributor

tagantroy commented Oct 5, 2017

@arturbosch already merged, can we close it or mark as ready?

@Mauin Mauin closed this as completed Oct 6, 2017
@Mauin Mauin added this to the RC5 milestone Oct 6, 2017
@Mauin
Copy link
Collaborator

Mauin commented Oct 6, 2017

Closed via #445

@lock
Copy link

lock bot commented Jun 20, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related topics.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants