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

DATACMNS-1126 - Add Kotlin constructor support. #233

Closed
wants to merge 4 commits into from

Conversation

mp911de
Copy link
Member

@mp911de mp911de commented Jul 26, 2017

We now discover Kotlin constructors and apply parameter defaulting to allow construction of immutable value objects. Constructor discovery uses primary constructors by default and considers @PersistenceConstructor annotations.

ClassGeneratingEntityInstantiator can instantiate Kotlin classes with default parameters by resolving the synthetic constructor. Null values translate to parameter defaults.

class Person(val firstname: String = "Walter") {
}

class Address(val street: String, val city: String) {

	@PersistenceConstructor
	constructor(street: String = "Unknown", city: String = "Unknown", country: String) : this(street, city)
}

We now discover Kotlin constructors and apply parameter defaulting to allow construction of immutable value objects. Constructor discovery uses primary constructors by default and considers PersistenceConstructor annotations.

ClassGeneratingEntityInstantiator can instantiate Kotlin classes with default parameters by resolving the synthetic constructor. Null values translate to parameter defaults.

class Person(val firstname: String = "Walter") {
}

class Address(val street: String, val city: String) {

	@PersistenceConstructor
	constructor(street: String = "Unknown", city: String = "Unknown", country: String) : this(street, city)
}
Copy link
Member

@odrotbohm odrotbohm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it make sense to hide the dedicated constructor discoverer in BasicPersistentEntity's constructor? That avoids the first one to run in the first place. Looks like the already existing PreferredConstructorDiscoverer got a Kotlin code path as well now.

Also, I wonder If we can't have a dedicated Kotlin-aware EntityInstantiator to more cleanly separate that from the standard one. We should be able to detect that it's a Kotlin class we have to deal with and the just select the right EntityInstantiator in the first place, right?

@mp911de
Copy link
Member Author

mp911de commented Jul 27, 2017

I'd turn PreferredConstructorDiscoverer into an interface with static factory methods and enums for the particular implementations (Java and Kotlin).

An EntityInstantiator for Kotlin classes is mostly the same as an instantiator for Java classes. Only Kotlin constructors with default arguments require a more specific instantiation. We could create a subclass ClassGeneratingKotlinEntityInstantiator extending ClassGeneratingEntityInstantiator and differentiate in EntityInstantiators which one to use.

Split preferred constructor discovery in enum types. Refactor PreferredConstructorDiscoverer to interface with static methods for constructor discovery. Adapt tests.
Refactor ClassGeneratingEntityInstantiator to ClassGeneratingEntityInstantiator without Kotlin-specifics and ClassGeneratingKotlinEntityInstantiator extending ClassGeneratingEntityInstantiator.

Apply intantiation argument caching to ClassGeneratingEntityInstantiator.
odrotbohm pushed a commit that referenced this pull request Aug 25, 2017
… Kotlin.

We now discover Kotlin constructors and apply parameter defaulting to allow construction of immutable value objects. Constructor discovery uses primary constructors by default and considers PersistenceConstructor annotations.

KotlinClassGeneratingEntityInstantiator can instantiate Kotlin classes with default parameters by resolving the synthetic constructor. Null values translate to parameter defaults.

class Person(val firstname: String = "Walter") {
}

class Address(val street: String, val city: String) {

	@PersistenceConstructor
	constructor(street: String = "Unknown", city: String = "Unknown", country: String) : this(street, city)
}

Original pull request: #233.
odrotbohm pushed a commit that referenced this pull request Aug 25, 2017
Trailing whitespace in ReflectionUtils.

Original pull request: #233.
odrotbohm added a commit that referenced this pull request Aug 25, 2017
Moved PreferredConstructorDiscoverers into PreferredConstructorDiscoverer to hide more API. Renamed ClassGeneratingKotlinEntityInstantiator to KotlinClassGeneratingEntityInstantiator.

Original pull request: #233.
@odrotbohm odrotbohm closed this Aug 25, 2017
@odrotbohm odrotbohm deleted the issue/DATACMNS-1126 branch August 25, 2017 11:26
mp911de pushed a commit that referenced this pull request Aug 25, 2017
Moved PreferredConstructorDiscoverers into PreferredConstructorDiscoverer to hide more API. Renamed ClassGeneratingKotlinEntityInstantiator to KotlinClassGeneratingEntityInstantiator.

Use org.springframework.util.Assert instead of com.mysema.commons.lang.Assert.

Original pull request: #233.
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

Successfully merging this pull request may close these issues.

None yet

2 participants