I just completed my first decent-sized Kotlin project at GitHub - cretz/asmble: Compile WebAssembly to JVM and other WASM tools. Code feedback/review very welcome.
During development, I jotted down some things that annoyed me as I wrote it and I have collected them in this gist. Granted most are just my opinions and shouldn’t change the language, I just figured some feedback might be valuable. I hope the Kotlin devs take a quick peek even if they disagree w/ most.
Thanks for a great job. It is a great and exhaustive list. I’m sorry for narrowing this discussion to one specific item, but I have one specific clarification question. When you say in item 53 that “Either needs to be in the stdlib” do you actually mean a use-case for it as a union type, that is a disjoint unit of arbitrary types (lie
Int and in Scala’s doc on
Either) or your use-case is it to represent a pair of
Exception and some result (like Scala’s
Thanks a lot for your feedback! I’ve commented on the gist with responses to many of your issues; my colleagues will probably leave a few more comments.
My recommendation is for
Either<L, R>. I made a tiny one here. If you made it “right-biased” then a Try could easily be a
typealias Try<T> = Either<Throwable, T> I dunno. But yes, my use case was a union. Of course as an alterative, I would love actual union types, e.g.
String|Int like what Scala.js and Ceylon do, but that’s a bit radical.
Thanks a lot for explanation! I’ve looked through the code, too. Let me make some comments, if you excuse me.
I’ve looked at your usages of
Either. You are indeed using it as a union type on your own types. It does seem from the cursory reading of your code that it can be slightly improved. Let us take one example where you return
Either<Node.Import, Node.Global> from
globalAtIndex. In this particular example you can define an interface
ImportOrGlobal that is implemented by both
Node.Global and return that one instead of
Either. It looks like the immediate uses for this function can be made slightly simpler and easier to read after this change. The same seems true for other uses of
Either in your project.
Also, I’ve looked at some of the usages of
Triple in your code base. Many of them would defenitly benefit from one-line declaration of the
data class. For example, you could declare
data class FuncSig(val nameMap: NameMap, val exprsUsed: Int, val sig: List<Node.Type.Func>) right before
toFuncSig function. Not only this will immensly improve readability of the
toFuncSig function declaration itself, but this would also reduce the need to redeclare (by destructuring) the names of the actual components at every use-site of the corresponding function. The last, but not the least, it will make it easier to refactor the code in the future (e.g. adding a new returned item will not force all prior uses to be changed).
I do suggest you to try it, if you care. This will make your code more idiomatic from Kotlin standpoint.
I didn’t want an ImportOrGlobal because you have to go to the definition site to add an interface and it’s not my AST’s fault that I can’t represent a union in another package. Also with your interface idea I have now just lost my exhaustiveness checks in my
when statements and now always have to add an
else because there are no sealed interfaces. For other things I do have it like for parse results that represent a success or error. Arguably I could use exceptions here, but I choose to represent it as a possible result state. I could go through the Kotlin/IntelliJ code bases and say that that all uses of
Pair should be changed to use a
ThisAndThat class. We recognize the value of shortcuts, whether it’s composing two types or unioning them.
I agree about uses of Triple. I take the shortcut in these cases. I take the shortcut in a lot of cases. You’ll see I declare a data class in a lot of cases and I am aware of the choice. Quite often I choose pair or triple for return value tuples that I expect immediate destructuring by the caller. Another place these types of generic tuples are valuable is in local code when doing something like a
fold where you keep immutable state and return one or the other, one and the other, etc when you have multiple stateful representations at once. Granted, you can make your points for Pair and Triple in just about every place they are used in anyone’s code.
Good point about exaustivness checks. Thanks.