Kotlin / kotlin-style-guide

Work-in-progress notes for the Kotlin style guide
288 stars 14 forks source link

Using objects #6

Open yole opened 8 years ago

yole commented 8 years ago

Use top-level objects only if the instances of those objects will be passed somewhere (in other words, if the objects extend a base class or implement an interface). Do not use objects purely as a namespacing mechanism. Instead, use top-level functions and properties. To provide grouping, put stuff into multiple files, and use different package statements for files in the same directory.

voddan commented 8 years ago

Using objects with internal state is a valuable option

cypressious commented 8 years ago

I disagree with "not using objects as a namespacing mechanism". Sometimes there are functions that are only relevant in certain situations, for example operations on common collection classes. Making them extension methods on this class would pollute the autocompletion all over the module. Also, since there is no package private visibility, you can't restrict the scope of the functions if you also want to use them across multiple files.

Example from a fictive Android project (Bundle is a framework class for passing arguments, similar to a Map):

object OrderUtils {
    val KEY_CUSTOMER = "customer"
    fun getCustomer(bundle: Bundle): Customer? = bundle.getParcelable(KEY_CUSTOMER)
}

vs

val KEY_CUSTOMER = "customer"
fun Bundle.getCustomer(): Customer? = getParcelable(KEY_CUSTOMER)
yole commented 8 years ago

I think that polluting autocompletion is an IDE design issue, not a style guide issue. Getting rid of meaningless classes which do nothing but hold a bunch of static methods was a very explicit design goal of Kotlin. The fact that the current behavior of autocompletion pushes you back against that design goal is unfortunate, but it needs to be fixed in the IDE.

damianw commented 8 years ago

Fixing it in the IDE would be optimal solution, but would that allow developers to still explicitly namespace code? I feel like the lack of a package-private and package-protected visibility is the underlying problem, as @cypressious said.

As long as those features aren't on the roadmap, how else can visibility be restricted? Annotations are the only thing that comes to mind:

@file:AutoImportVisibility(PACKAGE_PROTECTED)
package foo
// ...

Not ideal, but I'm curious as to how you see "fixing the IDE" working.

yole commented 8 years ago

@damianw The fix I have in mind is simple. Completion initially shows only the extension functions which you have already imported. If you press Ctrl-Space for the second time, or if you type a prefix that matches a function you haven't imported yet, then you start seeing non-imported functions too.

matklad commented 7 years ago

What about namespacing at the call site? In python, you can design libraries for semi-qualified imports:

import formats.json

json.parse("{}")
json.to_string(True)

In Kotlin, this could look like parseJson("{}") / jsonToString(True) which is ok if you have only a couple of functions. But sometimes there are dozens of related functions, and you do want to provide some namespacing mechanism at the call site.

The example I have in mind is RsPsiFactory in the Rust plugin. Currently, it's an almost useless class which "partially applies" methods to the proejct argument. I would love to get rid of the class and just use free standing functions like crateFoo(project, text), but I would love to use them as psiFactory.createFoo for better readability and discoverability.

raderio commented 6 years ago

So, if we need to put related methods in one place we should create a new package/directory to put them there? For example we have EmailService what have a few methods to send email, we make it an object. So, according to style guide we need to create a package emailservice and put those function there?

yole commented 6 years ago

@raderio Yes, this is correct.

raderio commented 6 years ago

I think an example of this is https://github.com/JetBrains/kotlin/tree/master/libraries/stdlib/common/src/kotlin

raderio commented 6 years ago

in other words, if the objects extend a base class or implement an interface

But why in this case we even need object, instead we can use anonymous class

interface Stringify {
    fun str(): String
}

val foo = object : Stringify {
    override fun str(): String = "foo"
}

fun main(args: Array<String>) {
    println(foo.str())
}
MarcinMoskala commented 5 years ago

@yole Object declarations are used as packages even in the standard library, and this is a very important mechanism. Property delegates are placed in the Delegates object declaration. In the kotlinx.coroutines library dispatchers are collected in the Dispatchers object declaration. In both cases this class in important. Let's say that you see in some class the following:

class SomeClass: Superclass() { 
    // ...

    fun userObservable() = observable(user, someFunction)
}

What is that? It can be anything but it looks like some observable from SoleClass or its superclass. When the object name is forced, everything is clear:

    fun userObservable() = Delegates.observable(user, someFunction)

Same with IDE tints. If observable would be defined in top-level, it would be suggested everywhere. You might accidentally use it thinking that this is a superclass method. When it is in Delegates we know that this is the observable delegate from stdlib.

cbruegg commented 5 years ago

This would be less of an issue if we could import packages. Say we've got

package kotlin.delegates

fun observable(...): ... = ...

Then it would be interesting if we could use it like this:

import kotlin.delegates

class SomeClass {
  fun userObservable() = delegates.observable(user, someFunction)
}

There may also be big flaws in this idea.

MarcinMoskala commented 5 years ago

@cbruegg Well, in the current state you can, if you really want. Using import alias:

import kotlin.properties.Delegates as delegates
val i by delegates.observable(2) { property, oldValue, newValue ->  }

You can also omit this type if you want, using direct import:

import kotlin.properties.Delegates.observable
val i by observable(2) { property, oldValue, newValue ->  }

The point is that you are safe from accidental use.