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

Consider Kotlin extension-method "overloads" to StandardSubjectBuilder.that #660

Closed
cpovirk opened this issue Feb 4, 2020 · 4 comments
Closed
Labels
P3 not scheduled type=addition A new feature

Comments

@cpovirk
Copy link
Member

cpovirk commented Feb 4, 2020

Everywhere we expose a Subject.Factory (or CustomSubjectBuilder.Factory), we could expose a that method. Such methods would save users of assertWithMessage, check, expect, etc. (maybe including ExpectFailure in the Subject's own tests :)) from having to call about(foos()).

@tuanchauict
Copy link

tuanchauict commented Mar 21, 2021

Hi,
(I'm not sure I fully understand this issue, but I feel my idea is close to this.)

When I try to create a new Subject (in Kotlin), I feel it's not straightforward. There are some questions prompted in my head like:

  • What is FailureMetadata? how I can create that object? do I need to create that object?
  • How can I create assertThat? where should I put my assertThat for my subject?
  • With the Employee sample , do I need to care employees() if I just need to care assertThat()?
  • etc.

I suggest a new way:

abstract class SubjectFactory<S : Subject, T>(
    subjectConstructor: (metadata: FailureMetadata, actual: T?) -> S
) {
    private val subjectFactory: Factory<S, T> = Factory<S, T>(subjectConstructor)

    fun assertThat(actual: T?): S = assertAbout(subjectFactory).that(actual)
}

Then for a new subject, we just need:

class XyzSubject(
    metadata: FailureMetadata,
    private val actual: Xyz?
) : Subject(metadata, actual) {
    // Snippet

    companion object : SubjectFactory<XyzSubject, Xyz>(::XyzSubject)
}

Disclaim: today is the first day I try Truth but I love it

@cpovirk
Copy link
Member Author

cpovirk commented Mar 7, 2023

Thanks, @tuanchauict. I finally had a detailed look at your suggestion. I think it would be the best approach for someone who wants to use XyzSubject from Kotlin only. Inside Google, though, we're still usually interested in making our code usable from both Kotlin and Java. And to do that, we want to put @JvmStatic on the Factory accessor and assertThat method, as in this example. That doesn't appear to be possible with the abstract class SubjectFactory approach.

All that said, we know people who are looking more at pure Kotlin projects, so we may eventually want to provide exactly what you're suggesting for them.

@tuanchauict
Copy link

I hope Jetbrains will allow us to make @JvmStatic possible for the companion object. Here is a not-perfect workaround: https://pl.kotl.in/vcn1vZva9

@cpovirk
Copy link
Member Author

cpovirk commented Apr 3, 2023

Thanks, I might have given up before trying open fun.

The other thing I note about that implementation is that users would need to define a @JvmStatic property for the Factory, too, in order to give it a conventional name like "foos" rather than "subjectFactory" ("getSubjectFactory()" to Java users). This would all be so much easier in pure Kotlin! At least the current boilerplate is still better than what we have to write in Java.

Backing up to my original proposal:

we could expose a that method

Wait, no, we can't. Or we could, but it wouldn't do any good: Kotlin always prefers any member function that exists over any extension function. So, because StandardSubjectBuilder already defines some that methods, any fun StandardSubjectBuilder.that(...) that someone might define would be ignored. Again I say: This would all be so much easier in pure Kotlin!

@cpovirk cpovirk closed this as completed Apr 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 not scheduled type=addition A new feature
Projects
None yet
Development

No branches or pull requests

2 participants