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

Migrate code to Kotlin #242

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Migrate code to Kotlin #242

wants to merge 3 commits into from

Conversation

vlsi
Copy link
Owner

@vlsi vlsi commented Oct 19, 2021

Most of the conversion was automatic, so there might be glitches.

@vlsi vlsi force-pushed the kotlin_tests branch 2 times, most recently from 3e37d48 to b4dc108 Compare October 19, 2021 20:32
@vlsi
Copy link
Owner Author

vlsi commented Oct 19, 2021

@Pitterling, any thoughts?

@vlsi vlsi changed the title Migrate tests to Kotlin Migrate code to Kotlin Oct 20, 2021
@vlsi vlsi force-pushed the kotlin_tests branch 8 times, most recently from 99a32ec to d930008 Compare October 21, 2021 07:38
@Vest
Copy link
Contributor

Vest commented Nov 7, 2021

May I kindly ask, what is the benefit of this migration?

@vlsi
Copy link
Owner Author

vlsi commented Nov 7, 2021

Null safety, better stdlib for collections, etc

@Vest
Copy link
Contributor

Vest commented Nov 7, 2021

Ok, I expected something different that is specific to this project particularly. To me Kotlin has different a knowledge curve, longer compilation time, and IDEA IDE as a prerequisite.

@vlsi
Copy link
Owner Author

vlsi commented Nov 9, 2021

I expected something different that is specific to this project particularly

Frankly speaking, I do not think there's "the killer feature which makes Kotlin the only solution".

However:
a) Every time I see a reference field in Java I tend to question myself: "is null allowed there?". Kotlin resolves that question by making unchecked null dereferences a compile-time error
b) With Java, there's no "standard Java code formatting". There are lots of different formats, and often Checkstyle is used to fail builds when tab vs space is misplaced. With Kotlin, there's ktlint that automatically formats incorrect code style (thanks to the official Kotlin style guide), so gradlew style task is possible that would automatically format code (like insert missing indent) rather than fail the build with "look, you missed a space here". Frankly speaking, I am tired of checkstyle build failures, so removing checkstyle is a huge win for me. Note: I still think that there should be a common code style for the project, so it is not like I just drop checkstyle, but I am going to replace it with ktlint + autostyle for a way better dev experience.
c) UI programming in Java is ugly. Kotlin enables libraries like https://github.com/edvin/tornadofx or even https://github.com/JetBrains/compose-jb. Of course, ksar is not UI-heavy, however, theres' chicken-and-egg problem: UI in Java is non-trivial, so there's no fun in adding UI features.
d) Kotlin has lots of data manipulation methods in the stdlib. For instance, methods like iterable.filter, iterable.associate, and so on. If you need to convert list to map, group values, find an element, replace keys, Kotlin has all of that. It is way easier to both read and write than the corresponding Java's Stream API. Of course, I know there's https://github.com/amaembo/streamex that implements some of the missing features in Stream API, however, I do not care if my code is slightly less efficient. At the end of the day, ksar is not the application that should sustain 10 million requests per second. What I care more about is that I can extend it, I can understand the code, and so on.
e) Small things like string interpolation help as well. Can I live without string interpolation? Of course, I can. However, if I can use it appropriately, it makes the code way easier to read and maintain. For instance, see how IEEE1541NumberTest is easier than its previous Java variation.

I just do not want to read and maintain code that looks like https://github.com/openjdk/jdk/blob/739769c8fc4b496f08a92225a12d07414537b6c0/src/jdk.compiler/share/classes/com/sun/tools/javac/platform/PlatformUtils.java#L44-L65
Every bit of it is puzzling. Of course, Java has ways to make that code look better, however, there's no way to make it clear what the code does :-/
At the end of the day, PlatformUtils has been reviewed and committed to openjdk/jdk.

Just in case, most of the current code in PR is auto-conversion from Java with minor cleanup, so it does not mean the current .kt files are "the best". For instance, I think it is worth replacing SAX (event-style) parsing of the configuration files with DOM-like parser that would read the document and convert it all at once. Currently, the parser is "too clever" (see XMLConfig class), and it is hard to tell which variables are null, which are not, etc. Would replacing SAX with DOM reduce the efficiency of the code? Of course, it would. Does it matter? I doubt so. It might even be that it is worth replacing XML configuration with Kotlin DSL so there's no need to parse XML in the first place.

I am sure there are cases when slightly less efficient code is good enough, so even if migration to Kotlin would increase compilation times I do not really care since speeding up the Kotlin compiler is in the priorities of the Kotlin team (see the roadmap ), and the compilation does include type verification (e.g. data types, null safety). So it reduces the number of tests I have to write manually. By the way, if tests are present, then the compilation time is less important as test execution still takes cycles.
For local development, there's incremental build.

So it is not like I am saying "ok, I want to play with Kotlin, let's ksar be the first project". I am sure moving to Kotlin would help the further ksar evolution.

@vlsi
Copy link
Owner Author

vlsi commented Nov 11, 2021

Here's a recent report on ~2x improvement in the compilation time for the new K2 compiler: https://youtu.be/db19VFLZqJM?t=2651

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants