mcarleio / konvert

This kotlin compiler plugin is using KSP API and generates kotlin code to map one class to another
https://mcarleio.github.io/konvert/
Apache License 2.0
93 stars 8 forks source link

Option to map multiple objects into one #71

Open mohsunb opened 5 months ago

mohsunb commented 5 months ago

Hi.

Today I needed to combine class A and B into C using Konverter. When having a method with A and B as parameters, compiler complains about multiple parameters. If I annotate either with Konverter.Source, the other one is completely ignored as if it didn't even exist.

This was fairly easy with MapStruct in Java, but I did not manage to replicate the same with Konvert. Is this something planned or not?

mcarleio commented 5 months ago

With Konverter.Source you can already pass additional parameters to a mapping function, which will automatically be recognized and used, see example below.

class SourceClass(val property: String)
class TargetClass(val property: String, val other: Int)

@Konverter
interface Mapper {
    fun toTarget(@Konverter.Source source: SourceClass, other: Int): TargetClass
}

As far as I understand, you would like to have something like this:

class SourceClass(val property: String)
class OtherSourceClass(val other: Int)
class TargetClass(val property: String, val other: Int)

@Konverter
interface Mapper {
    fun toTarget(source: SourceClass, otherSourceClass: OtherSourceClass): TargetClass
}

Currently this is not supported, but sounds like a legitimate feature request. There are definitely some challenges with that, but I will have a look.

mohsunb commented 5 months ago

Thanks for the answer! This actually helped me quite a lot. I will keep the issue open if you want to try implementing this feature. But no pressure. :)

stravag commented 5 months ago

Without understanding the exact use-case of @mohsunb I'd be weary of adding that feature to the library. What would be the expected behaviour if OtherSourceClass would evolve into:

class OtherSourceClass(val property: String, val other: Int)

I think it would require quite a bit of additional configuration in the shape of annotations to make it work. Wouldn't it be easier and more robust to let the caller handle it?

mapper.toTarget(sourceClass, otherSourceClass.other)
mcarleio commented 5 months ago

Yes, that is one of the challenges I also thought of. For this, I have some ideas, but they all come with downsides:

I am open to further suggestions and points I might have forgotten and/or should consider.

@stravag thanks for your PR with the added documentation and the unit test :+1: Appreciate that!

mohsunb commented 5 months ago

Hello.

One use case I often encounter at my job is to combine columns of two (or more) tables. As in, multiple JPA Entity classes into one DTO.

@stravag Regarding your concern of OtherSourceClass evolving over time. With MapStruct, if the source is ambiguous it would just break during compilation and ask for explicit source declaration. So, if the developer intends to change the OtherSourceClass, they have to adjust to mapping accordingly. Which, I think, is totally fair.

@mcarleio I agree with your first and third ideas since they are present in MapStruct as well and I am used to it, but that's just my opinion. The "black magic" you mentioned does not sound quite intuitive, I'd recommend against that.