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

Simplify Primary Constructor Properties #1734

Open
Janglinator opened this issue Nov 23, 2023 · 3 comments
Open

Simplify Primary Constructor Properties #1734

Janglinator opened this issue Nov 23, 2023 · 3 comments

Comments

@Janglinator
Copy link

Janglinator commented Nov 23, 2023

When attempting to add primary constructor properties, the solution is rather verbose. Considering our team abuses them, I'd like to propose a separate function that adds strictly a construction property, as can be denoted by the following extension:

fun TypeSpec.Builder.addConstructorProperty(propertySpec: PropertySpec): TypeSpec.Builder

Sample usage:

TypeSpec.classBuilder(className)
    .addConstructorProperty(PropertySpec.builder("dao", daoClass)
        .addModifiers(KModifier.PRIVATE)
        .build()
    )
    ....

I actually already implemented this with a hacky little workaround:

fun TypeSpec.Builder.addConstructorProperty(propertySpec: PropertySpec): TypeSpec.Builder {
    val initializerSpec = PropertySpec.builder(propertySpec.name, propertySpec.type)
        .initializer(propertySpec.name)
        .addModifiers(propertySpec.modifiers)
        .build()

    addProperty(initializerSpec)

    val retainedKey = TypeSpec.Builder::addConstructorProperty.name
    val specs = getRetainedValue<MutableList<PropertySpec>>(retainedKey) ?: mutableListOf()
    specs.add(initializerSpec)
    setRetainedValue(retainedKey, specs)

    val constructorSpec = FunSpec.constructorBuilder()

    for (spec in specs) {
        constructorSpec.addParameter(spec.name, spec.type)
    }

    return primaryConstructor(constructorSpec.build())
}

Where get/setRetainedValue are used to add instance variables in extensions. Note that it abuses the fact that we can invoke primaryConstructor multiple times.

I'm sure there are potential use cases where this wouldn't work or would lose values, but it seems stable enough for our use case.

@Janglinator Janglinator changed the title Simplify Primary Constructor Property Simplify Primary Constructor Properties Nov 23, 2023
@Egorand
Copy link
Collaborator

Egorand commented Nov 27, 2023

Agree with the premise - this is indeed a common use case that currently requires quite a bit of code. But the proposed implementation likely doesn't cover many possible use cases, in particular it's not apparent that the method creates a new primary constructor and attaches it to the TypeSpec builder. If there already was a primary constructor before addConstructorProperty was called, I imagine it'll be discarded? Perhaps we can require a FunSpec for the primary constructor to be created manually and passed to addConstructorProperty?

If you'd like to send a more fleshed out PR with unit tests that exercise different corner cases, it'd be interesting to see what the final API shape could look like. Something to consider here is that the final version might be more verbose and cumbersome to use than what you already have in your project, simply because it has to be more generic and handle all kinds of use-cases.

@Janglinator
Copy link
Author

As I become more familiar and confident with KPoet, I'll revisit this and consider leaving a PR. I'm just diving into the project at the moment.

In my particular implementation, previous constructor properties would NOT be discarded, they're stored using the setRetained/getRetained functions I mentioned. However, if primaryConstructor were called outside of this function, it would be discarded. As I mentioned, I kinda tossed this together just to experiment with the possibility.

@Egorand
Copy link
Collaborator

Egorand commented Nov 28, 2023

Fair enough, please do send us a PR!

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

No branches or pull requests

2 participants