Try to use more idiomatic kotlin

AzureMarker / intellij-lalrpop

Jetbrains plugin for the LALRPOP parser-generator

MIT License

16 stars 2 forks source link

Try to use more idiomatic kotlin #32

Closed dnbln closed 3 years ago

dnbln commented 3 years ago

So I went through https://github.com/yole/idiomatic-kotlin and tried to apply some of the examples there

AzureMarker commented 3 years ago

Nice! I found the slides from the talk (but no recording seems to be available): https://www.slideshare.net/intelliyole/idiomatic-kotlin

AzureMarker commented 3 years ago

I like most of the suggestions from the slides, except "Consider extracting non-essential API of classes into extensions". This is a bit overkill, and distributes the class's functionality across multiple top-level items (and potentially files). I only see downsides with this refactoring.

dnbln commented 3 years ago

Well that's the only one I didn't fully understand

Only change that came from that was moving parseError and errorRecovery out of LpTypeResolutionContext: https://github.com/Mcat12/intellij-lalrpop/pull/32/files#diff-4792a58af082c4f9334350a64e3e190ce8a5c57e97440cd1d077bac8072d10e8L45-L53

AzureMarker commented 3 years ago

I did also find LpMacroArguments.getSubstitution, but other than undoing that refactoring the PR is looking pretty good. I still need to look at it fully in detail, though I don't expect any more major issues.