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

convert source code to kotlin #3026

Closed
christophsturm opened this issue Jul 29, 2019 · 15 comments
Closed

convert source code to kotlin #3026

christophsturm opened this issue Jul 29, 2019 · 15 comments

Comments

@christophsturm
Copy link
Contributor

I think we should convert all source files that we change to kotlin and write new source files in kotlin.

This may seem a bit far fetched but:

  • I have done this on several projects already and it was always a very smooth transition
  • kotlin code is easy to read for java programmers.
  • idea can automatically convert java files to kotlin, without changing the semantics or api.
  • kotlin can be uses as just a "better java" while we transition, and we can refactor to more idiomatic kotlin if we desire
  • kotlin makes things like lombok obsolete, all kotlin objects automatically have getters and setters.
  • kotlin is from jetbrains so 1. it must be great and 2. it has great support in IDEA.
  • its basically java without boilerplate and with easier ways to write funtional code.
  • kotlin makes a lot of discussions from our style guide obsolete.

for example in bisq-network/style#12 instead of

if (a) return X
else if (b) return Y
else return Z

you can just write
return if (a) X else if (b) Y else Z

or for bisq-network/style#11 vals are automatically constant which makes the discussion about final or not for local variables obsolete

If people like this idea i would just change the gradle file to allow kotlin sources too, and then we can transition as fast or as slow as we want. My suggestion would be that anytime we do a significant change to a class we first convert it to kotlin via the convert to kotlin function. Also gui classes and classes that use lombok are primary candidates to be converted.

I think reading and writing kotlin is just more fun.

@freimair
Copy link
Contributor

The only way I see this happening is by having integration test in place. bisq-network/proposals#74

@christophsturm
Copy link
Contributor Author

i think converting the sourcode to kotlin on a file per file basis should not make any logic changes or require refactorings. Still we could start using kotlin for new code and for unit and integration tests.

@christophsturm
Copy link
Contributor Author

also we could start using kotlin for unit tests. there are really nice kotlin test libs
for example https://strikt.io which works with any test runner but has really great assertions.
I also like minutest https://github.com/dmcg/minutest but that could probably be too much for some people

@freimair
Copy link
Contributor

freimair commented Jul 31, 2019

converting the sourcode to kotlin on a file per file basis should not make any logic changes

yes, but the code looks different and a reviewer has to make a decision. I see that only happening when test coverage comes up to speed and therefore can take the heat off the reviewer.

Still we could start using kotlin for new code and for unit and integration tests.

agreed

@battleofwizards
Copy link
Contributor

I am super late to the project and feel free to ignore my suggestions.

That being said: Kotlin is great. And yet I suggest against converting to Kotlin.

By converting to Kotlin we expect newcomers to understand both Java and Kotlin.

Bisq is already a very demanding project: Java 10/12, JavaFx, advanced Bitcoin, security, p2p / Tor / anonymity, cryptography, "decentralized UX", and what not.

Even for advanced Java programmers, not everyone is keen to learn Kotlin. Now, if we also intend to attract experienced programmers in general (say Bitcoin experts knowing C# or Python who can pick up Java) - that is even worse.

To sum up: Kotlin is great but please do not further increase project complexity by requiring two languages.

@christophsturm
Copy link
Contributor Author

i just don't see how kotlin increases the complexity. c# and python experts will also have an easier time writing kotlin.

at its base kotlin is just java without boilerplate and encouraging immutability. and everybody is free to use as much kotlin as they want.

@Giszmo
Copy link

Giszmo commented Aug 29, 2019

I love Kotlin but your OP makes me cringe hard.

You suggest to replace

if (a) return X
else if (b) return Y
else return Z

with

return if (a) X else if (b) Y else Z

which is ugly as hell. Maybe you want to edit that to:

for example instead of

int getSomeValue(@Nonnull Boolean isA, Float b) {
    if (isA) {
        return 1
    } else if (b != null && b > 5) {
        return 2
    } else {
        return 3
    }
}

you can just write

fun getSomeValue(isA: Boolean, b: Float?) = when {
    isA -> 1
    (b ?: 0f) > 5 -> 2
    else -> 3
}

as slamming all in one line is not helpful for readability and thus for security and also {{}} are very important by my estimates for the if statements to avoid additional expressions to slip out of a context. Also my extension to your example addresses the null-safety and the simplicity of pulling the return statement out to not have it at all.

Edit: To be honest, the (b ?: 0f) > 5 -> 2 line took me a bit more than expected but I love Kotlin for its strictness. 0 is of type Int while 0f is of the (correct) type Float. While Java allows me to check if (b > 5) and throws if b is null, Kotlin doesn't allow this but if I am fine with the NPE, I can (instead of declaring b: Float) test for b!! > 5.

@Giszmo
Copy link

Giszmo commented Aug 29, 2019

By converting to Kotlin we expect newcomers to understand both Java and Kotlin.

Bisq is already a very demanding project: Java 10/12, JavaFx, advanced Bitcoin, security, p2p / Tor / anonymity, cryptography, "decentralized UX", and what not.

Java developers that started using Kotlin, don't want to go back. It's just so much better. So if we convert to Kotlin, we would not require Java anymore (although I assume the number of people familiar with Kotlin who don't know Java is negligible).

On the other hand, Kotlin doesn't only reduce boilerplate by a lot, it also has way superior null-safety.

Reduced boilerplate is security and null-safety is stability.

Lastly, conversion can be done very quickly and the reviewer can reproduce the conversion to reduce the diff to the functional changes.

I would highly recommend to at least allow new files to be in Kotlin and encourage to change files that are being worked on to Kotlin. Switching the whole code base to Kotlin would make it easier to review (3 devs do it and look at what differences they come out with, after resolving the trivial issues that normally result from automatic conversion. They would have to refrain from axing all the remaining boilerplate right away in a first big conversion.)

@blabno
Copy link

blabno commented Aug 29, 2019

As mentioned during dev call. If there is not enough developers who speak given language, then there won't be enough people to review that code.
There was the same argument for using Lombok: "It reduces the amount of boilerplate". But it turned out to be additional hurdle for new devs.
Java also offers null safety, just use @Nullable/@nonnull or similar annotations.

@battleofwizards
Copy link
Contributor

I recommend to stay boring. Boring Java. Boring cryptography. Boring dependencies.

We already have plenty of exciting and hard engineering problems in Bisq - decentralization, anonymity, security, correctness, Bitcoin, p2p, usability, the real non-scam DAO (!!).

I don't need exciting Kotlin to the mix ;) Enough is enough ;)

@christophsturm
Copy link
Contributor Author

its not about being exciting, its about mental load. the complexity of bisq is an argument for kotlin, not against it. complex code is easier to read in a more expressive language.

@stale
Copy link

stale bot commented Dec 3, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the was:dropped label Dec 3, 2019
@julianknutsen
Copy link
Contributor

I just wanted to give my 2c from the outside joining the project after this decision was made. New languages and frameworks need champions and adoption to be successful. I think this was identified in the dev call as well with the intention to try it out for a few months to see if the benefits outweighed the costs and overhead.

In the last 3 months, there has been only 1 commit using this new language

julian@dev:~/bisq$ git log --follow *.kt
commit 26c053dae80006338cb3df2e1ec6618cdcf58ff8
Author: Christoph Sturm <me@christophsturm.com>
Date:   Tue Aug 20 12:09:52 2019 +0200

    Apply kotlin plugin and convert one unused class to kotlin
    
    just to check that everything works and to show how much simpler
    kotlin code looks.

So regardless of whether or not it is better from a theoretical point of view, the reality is that other developers aren't utilizing the new language and using it for the benefits that were described. It is a good conversation for a future dev topic call.

bisq-network/events#31

@stale stale bot removed the was:dropped label Dec 6, 2019
@stale
Copy link

stale bot commented Mar 5, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the was:dropped label Mar 5, 2020
@stale
Copy link

stale bot commented Mar 12, 2020

This issue has been automatically closed because of inactivity. Feel free to reopen it if you think it is still relevant.

@stale stale bot closed this as completed Mar 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants