.toDataFrame fixes

[koperagen]

Fixes #1465
Btw i think strict ktlint_standard_function-expression-body rule is unnecessary, functions with return and with expression body can be a preference and coexist. At least for me this rule ALWAYS fails and forces me to rewrite

[AndreiKingsley]

Great, this logic is now really clear!
But please add a KDoc - or at least a comment for now, - to clearly define the behavior of toDataFrame; it's not actually trivial.

[AndreiKingsley]

Oh, that was weird, didn't know!

[Jolanrensen]

Btw i think strict ktlint_standard_function-expression-body rule is unnecessary, functions with return and with expression body can be a preference and coexist. At least for me this rule ALWAYS fails and forces me to rewrite

I have to disagree. Adding { return } for me feels like a waste of space, and mixing both expression functions and single-line-return functions in the same file feels messy.
You, for instance, wouldn't mix lambdas and normal function definitions in one place either, even though they do the same thing.

val myFunction: (String) -> Unit = { arg1 ->
    doSomething()
}

fun myFunction(arg1: String) {
    doSomething()
}

It increases cognitive load. I'd be fine if they were all single-line-returns too, but we're a JetBrains Kotlin library, so if we don't follow the guidelines, then who does? :P

But I understand it can be annoying if it fails the build :) For that I can recommend installing the KtLint plugin in the IDE and binding it to the reformatter, Ctrl+Alt+L. It highlights these cases and can fix them with one push of a button, or even upon file-save. Personally, I use it with pleasure, and as a result, the ktLintCheck almost never complains even though I often forget commas, write too long lines etc. And yes, sometimes it's a bit of a back-and-forth between me and the linter, but in the end the code looks clean, consistent, and readable.

[koperagen]

It increases cognitive load. I'd be fine if they were all single-line-returns too, but we're a JetBrains Kotlin library, so if we don't follow the guidelines, then who does? :P

I agree with the following

fun foo(): Int {     // bad
    return 1
}

fun foo() = 1        // good

However in Kotlin anything can be a single expression:

fun foo() = run { 
  // multiple statement go here!
}

Key point why i'd like that function to have a body: a lot of lines of code in the expression. We can keep this rule but split expressions into multiple statements, and technically linter won't complain
For trivial cases when there's only single line or otherwise not much code, always use it sure! Or if you want to rely on type inference, might as well abuse run { } - with Kotlin DataFrame compiler plugin for example.

[Jolanrensen]

can we deprecate this function here? since it doesn't always work

[Jolanrensen]

good increase in robustness :)

[Jolanrensen]

Turning this off project-wide disables the quick-fix too. I use this quite often :( I'd rather recommend suppressing it on a class/file/function basis if you really need it for clarity. The KtLint plugin has a quick-fix option for suppressing.

[Jolanrensen]

I'd either rewrite it declaratively/functionally, like:

private fun KClass<*>.properties(): List<KCallable<*>> =
    memberProperties
        .filter { it.visibility == KVisibility.PUBLIC }
        // fall back to getter functions for pojo-like classes if no member properties were found
        .ifEmpty {
            memberFunctions
                .filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() }
        }

or imperatively:

private fun KClass<*>.properties(): List<KCallable<*>> {
    val publicMembers = memberProperties.filter { it.visibility == KVisibility.PUBLIC }
    return if (publicMembers.isEmpty()) {
        // fall back to getter functions for pojo-like classes if no member properties were found
        memberFunctions.filter { it.visibility == KVisibility.PUBLIC && it.isGetterLike() }
    } else {
        publicMembers
    }
}

(or a variant of either). In either case the linter won't complain while sticking to one code paradigm.

[Jolanrensen]

nice! We should really do a sweep of ::class usage in DF. typeOf<T>() is cheaper I believe

[Jolanrensen]

does it work as well for interfaces, data classes, and objects?

[koperagen]

No for data classes - they require at least one property. But in general anything T that has no member properties / getter like functions, here we rely on Kotlin reflection