Hi guys, I was trying out testing builders and I h...

# apollo-kotlin

[Jan Skrasek]

Hi guys, I was trying out testing builders and I have to admit that I got burned a bit.
This is what I have to have:

val data = UserDataQuery.Data {
  viewer = userViewer {
    userId = "userId"
    identity = identity {
      email = "<mailto:[e-mail]|[e-mail]>"
      name = name {
        first = "Jon"
        last = "Snow"
      }
    }
    preferredTrips = preferredTrips {
      edges = listOf(edge { node = node { kind = "MUSIC" } })
    }
  }
}

This was my first attempt:

val data = UserDataQuery.Data {
  userViewer {
    userId = "userId"
    identity {
      email = "<mailto:[e-mail]|[e-mail]>"
      name {
        first = "Jon"
        last = "Snow"
      }
    }
    preferredTrips {
      edge { node { kind = "MUSIC" } }
    }
  }
}

Have you considered something like this?

[mbonnin]

Hi 👋

[Jan Skrasek]

(A side note: current solution would utilize CheckResult annotation from Androidx to see this directly linted as not used result, though I understand that in multiplatform this is problematic.)

[mbonnin]

Right

[mbonnin]

I can see how the first version is more concise

[mbonnin]

But also kind of breaks the symmetry...

[mbonnin]

Maybe we can make it so that if userViewer is called and not stored into one field, it throws a runtime error

[mbonnin]

For the list, how would you like to handle multiple items? Something like this?

preferredTrips {
      edge { node { kind = "MUSIC" } }
      edge { node { kind = "VIDEO" } }
    }

[Stylianos Gakis]

I think this is an important detail. I like the way these builders are explicit about which field is getting populated by setting identity = identity {...}. I am not sure if the ambiguity would be worth it with the other approach. Just throwing my opinion out there 😅

[mbonnin]

The unused return value is definitely a footgun but if we can detect it early enough that should be alright 🤞

[Jan Skrasek]

https://kotlinlang.slack.com/archives/C01A6KM1SBZ/p1652176227634479?thread_ts=1652175546.439879&cid=C01A6KM1SBZ
Yes, I expected this.

[bod]

(Related: I've created this issue asking for a check like the one that exists in Java)

[Jan Skrasek]

I guess I expected more something like this: https://kotlinlang.org/docs/type-safe-builders.html
Of course, now I know how to use it, there is not much of issues .... 😄 Yet I hoped for less of the code.

[Jan Skrasek]

There is already an issue for check result:
https://youtrack.jetbrains.com/issue/KT-12719/Add-CheckResult-MustUseReturnValue-to-warn-about-unused-return-v

[bod]

Woops thanks! I searched before creating the issue, I swear 😅

[mbonnin]

And I opened this one to fail on unused return values in test builders https://github.com/apollographql/apollo-kotlin/issues/4093

[mbonnin]

Issue galore \o/

[Stylianos Gakis]

A great (imo) update on this issue https://youtrack.jetbrains.com/issue/KT-12719.
Another win of doing the right thing by default and allowing opt-in for the alternative in some way , like the decision was made for when statements to be exhaustive by default (annotation here, else -> in when statements).
But Roman also said that this probably isn’t gonna happen anytime soon “we just don’t have short-term resources to execute it” so I wouldn’t hold my breath waiting.

[bod]

oh yeah I saw this! Too bad this is for later - we need this NOW 😛

in related news, yesterday I took a look at making it a runtime error to not assign a field in the test builders dsl. This should be in the next release.

[Stylianos Gakis]

Wow you fellows are too fast on fixing everything, I can’t keep up 😅

[bod]

wow I think this formatting is done automatically by KotlinPoet and probably due to the long identifier which now makes the line longer than whatever its max line length is! But yes I'm surprised at the Kotlin compiler not liking this here 🤔

[Stylianos Gakis]

Is this pure KotlinPoet formatting?
Is there no formatter or something that passes through the generated code on the apollo-kotlin side?
If so I guess this is a problem for them to solve, and it should be reported, but I would be very surprised if they hadn’t caught this before. It seems like it is something that can happen very easily with such long names

[bod]

yes this is purely KotlinPoet but there's actually a fix: you can use non breaking spaces in the code specification 🙂

[bod]

https://square.github.io/kotlinpoet/#:~:text=Unfortunately%20this%20code,to%20our%20example

[bod]

so that's missing in this case, I can make a quick fix 🙂

[Stylianos Gakis]

Big brain solution 🧠
Nice thank you a lot 🙌

[Stylianos Gakis]

Btw this makes me thing, this would make me at least a bit paranoid. Like having to add the special spacing character instead of normal space in every place where such code is generated. Is this what you’re gonna do now, or just for this specific case? I am not sure what’s best practice when using KotlinPoet

[bod]

haha good thinking! I was about to add it in most return cases. We've been using the character in some places but not all...

[Stylianos Gakis]

Such is the life of a Kotlin Poet I suppose 🧑‍🎨

[bod]

hahaha 😄 well said

[bod]

here we go

[Stylianos Gakis]

Thanks a lot, again 🥳

[bod]

happy to help!