-
Notifications
You must be signed in to change notification settings - Fork 363
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
Guards #371
Comments
Instead of the |
Unfortunately, this is not as easy. For example, if I write: fun foo(n: Int) = when (n) {
0, 1 -> "a"
otherThing() -> "b"
} it's not clear whether It could be possible to make a "better" transformation by checking the types, but I think that in this case having this translation hold before the type checking phase is a good thing to have. |
I would expect
Hence, any main condition in a when-with-subject that isn't preceded by |
While the idea behind this proposal makes sense to me, I don't think it translates into reality well. It ends up being shackled by some confusing rules that could just make the resulting code less clear rather than more. In the absence of exhaustiveness checking, the only real remaining motivation for the change is the argument that when-with-subject is more expressive and concise.
Actually I disagree with that argument. The proposed syntax: when (status) {
is Status.Ok && status.info.isEmpty() -> "no data"
...
} seems to me to be more confusing than just using a when {
status is Status.Ok && status.info.isEmpty() -> "no data"
...
} This is only a few characters longer than the proposed modification, behaves exactly the same, and requires zero additions to the language. If I want to make it clear that "status" is the subject, I can do so with status.let {
when {
it is Status.Ok && it.info.isEmpty() -> "no data"
...
}
} I understand the concerns that have led to the restricted version of the guard syntax in the proposal. But those restrictions are new rules to be learned, and to be honest, I already find the left hand side of a when-with-subject to be a confusing deviation from the ordinary rules of the language. The additional restricted guard syntax makes it more confusing. Without the benefits of exhaustiveness checking, I don't think this proposal can meet the minus 100 points bar. Even if exhaustiveness checks could be added, I worry they would become complex and hard to reason about. |
Although this proposal doesn't concern with exhaustiveness checking, we are pretty confident that if we implement this in the Kotlin compiler, the exhaustiveness would be correspondingly upgraded. |
Thanks, that's good to know! From the linked ticket, I gather the proposed improvements also extend to There's no need for two ways of doing the same thing. Once you add exhaustiveness checks, |
@roomscape I personally think when-with-subject has great potential to be a powerful pattern-matching construct. In its current form, it isn't yet, but I think it can get there, so I don't think it's a good idea to drop it entirely. |
I think this proposal makes sense only with exhaustiveness checking. As an android dev I write code similar to the provided examples quite often. And the reason I do sealed classes for states that are mutually exclusive is because I don't have to worry about adding a new subtype and forgetting about updating the when statement. |
Heavily agree with @roomscape 's first reply. In addition, I have three things
fun Status.renderNested(): String = when(this) {
is Status.Loading -> "loading"
is Status.Ok -> if(info.isEmpty()) "no data" else info.joinToString()
is Status.Error -> when(problem) {
Problem.CONNECTION -> "problems with connection"
Problem.AUTHENTICATION -> "could not be authenticated"
Problem.UNKNOWN -> "unknown problem"
}
} |
Indeed, in smaller examples it's arguable that the change is minimal, and as argued in the KEEP, the translation is also quite direct to non-subject version. However, this has the potential of steering the people towards a style where nested control flow is made clearer. Some use cases where I think guards shine:
private fun FirExpression.tryToSetSourceForImplicitReceiver(): FirExpression = when (this) {
is FirSmartCastExpression ->
this.apply { replaceOriginalExpression(this.originalExpression.tryToSetSourceForImplicitReceiver()) }
is FirThisReceiverExpression && isImplicit ->
buildThisReceiverExpressionCopy(this) {
source = callInfo.callSite.source?.fakeElement(KtFakeSourceElementKind.ImplicitReceiver)
}
else -> this
} |
If |
I'm concerned about using the existing token Using any other unambiguous token, including a new keyword or a new symbol combination, would be much more preferable. It will have to be learned by users, but that is a good thing. |
For exhaustiveness checking, I think a current hole would be classes like |
Unfortunately this is also the case currently with Kotlin, as data flow analysis works left-to-right. For example, |
Uniformly using |
Yes. This is why I mentioned "no side effects, no type information from the left is used on the right, etc". |
That is the problem. The natural syntax would be
which is just two letters more (or three if the subject is nullable) and does not need explanation. |
Or, for the lazy, it could be
But maybe that is too compact. |
Should the proposal ensure that less specific conditions should go after more specific ones? In other words, the following should better fail at compile time: when (str) {
is String -> println("String")
is String && str.isNotEmpty() -> println("non-empty String") // it is never reached
} |
Would like to note, when (charSequence) {
is String && charSequence.length > 5 -> ...
"MEOW" -> ...
else "meow" in charSequence -> ...
else -> null
} For elses, we can just not put a guard keyword at all. Oh, and another thing: IntelliJ should have the option of coloring the |
This is one of the reasons why I dislike |
The KEEP has been updated with sections on why |
This should be covered in principle by the reachability analysis in the compiler. |
We have preliminary results from our survey (which cannot be shared directly, as it contains some personal information). The comments, and the discussion that they've spawned within the Kotlin team, have pointed some weaknesses of the original proposed syntax with As a result, a new version of the proposal has been published, using |
While reading the initial proposal, I found that it might collide with future pattern matching features. As guards with The syntax |
The KEEP has been merged. Thanks everybody for their comments and suggestions! |
@serras , most people became aware of this feature after this issue was closed. Is this feature still open to feedback or has it been set in stone? Kotlin has been my primary language since 2017 and I trained several teams on transitioning to Kotlin and coached countless individuals on Kotliln. The idea of guarded conditions is nice with some clean implementations in other languages. However, the current syntax and extra capabilities in the KEEP are a bad fit when taken together with existing nuances and mental model of Kotlin. I think that guarded conditions are nice to add when aligning with the pre-existing mental model but in the current form, they degrade the overall quality of the language. Am I too late to this discussion or should I elaborate more? |
I'd say the downsides of this feature are important even if implemented as-is -- even if just for Kotlin 3.0's sake. |
@daniel-rusu the current design is going to be implemented as described here for 2.1. This is going to come in Beta, so there's still room for additional improvements, so feel free to elaborate here; but bear in mind that at this point we may not take big U-turns in the design. |
I too feel this was a bit rushed. KEEP was opened in February and merged in May. I presume this was due to KotlinConf. The problem is that there are still important issues to be discussed, such as @vlsi's comment or the exhaustiveness topic. Both cases introduce pitfalls to the language, and on the other hand, the gains doesn't look to compensate such issues. Any chance this can be implemented as experimental, just as with context receivers? |
Yeah, this was very rushed. JetBrains have never sacrificed language quality for marketing, and I hope this is an exception and not the rule. Languages, after all, are FOSS, and can be forked. And if JetBrains starts Microsoft-ing/Oracle-izing Kotlin, it would just be hard forked by the community. This is not a threat (who am I to maintain a language), but am just pointing out facts - it's not worth it for JetBrains to prioritize marketing over language. It'll result in tech debt and a dissatisfied userbase, which a large part of it cares enough to stop using JetBrains tooling completely (considering Kotlin's entire selling point is "Java/JS but good and without oracle/MS"). So, back to a more on topic topic - question: why guards at all, actually? when (any) {
is String if length == 7 -> { /*...*/ }
is String -> { /*...*/ }
else -> { /*...*/ }
}
// Completely equals to
when (any) {
is String -> if (length == 7) {
// ...
} else { /*...*/ }
else -> { /*...*/ }
} This is much more readable, and is a matter of modifying the official code style, instead of adding a controversial language feature. The new K2 compiler is able to do smart casting and other flow checking even without new language features. And a short if, with no braces, would be even more readable. Nested whens, too, are even more readable - could implement a warning to use a The only bonus of guards is the ability to skip; i.e. only have when (any) {
is String -> if (length == 7) /*...*/ else continue
is CharSequence -> { /*...*/ }
else -> { /*...*/ }
} It feels much more Kotlin-y for me to support |
I'll explain my concerns and sorry in advance for the length. I'm not trying to stop the concept of this feature but instead hoping to nudge the direction into a form that achieves the same goals but with fewer sacrifices. I'll take an analytical data-based approach to steer the conversation away from personal preferences. From the Kotlin specification, one of the main goals of Kotlin is to be pragmatic for developers focusing on getting work done essentially making developers more productive: https://kotlinlang.org/spec/introduction.html It's well-accepted in the industry that the ratio of time spent reading versus writing code is about 10:1. This suggests we should gladly trade a significant increase in writing effort for a small reduction in reading effort. If a change increases the overall writing time by 100% but cuts the overall reading time by just 20%, productivity improves (eg. 10 hrs reading + 1 hr writing turns into 8 hrs reading + 2 hrs writing thus saving 1 hour). DSLs are a great example of this tradeoff where they can be a bit tricky to define but make the intention of the code that uses them obvious. The effort in reading code is due to the cognitive complexity of understanding the meaning rather than the time it takes to read the text. We usually skim through code very quickly before deciding where to slow down and focus so we quickly move past 95% of code making quick assumptions along the way. When our assumptions are incorrect, we can spend lots of time going in the wrong direction. Therefore the code shouldn't just be understandable when read carefully, instead, it should be intuitively obvious, especially at the language level so we can skim through it quickly. Given that Kotlin has already reached a stable state, to test whether a new feature is readable, the majority of people who have never heard of this feature should be able to briefly look at a snippet of code that uses most of the abilities of the new feature and intuitively assume the correct meaning without having to take time to think about it or read any explanations of how this feature works. If this bar cannot be met then the cognitive effort is increased and readability is reduced. We might think that new capabilities that reduce defect rates save us from the reading time it would take to work on those defects thereby improving productivity. This is true when preventing common categories of defects like null pointer exceptions. Considering that NPEs are 1 category of defects, several years ago I counted 50 categories of defects that were possible in Java but were prevented by Kotlin language features. In Java, we spent lots of time hunting down programming errors and carefully analyzing the possibility of mistakes. Kotlin features guarantee that most of those can't happen so we don't have to think about them. In Kotlin, we're usually hunting down misunderstandings or scenarios that we haven't considered, so the code is usually behaving as intended but our intentions were incorrect. Since Kotlin already prevents so many categories of defects, most of the low-hanging fruit has been picked. There are some promising candidates like name-based destructoring, but for the most part further defect reduction would be targetting a tiny portion of the overall defects that Kotlin developers commonly encounter. Reducing defect rates is welcome but this by itself would likely have a minimal impact on our overall productivity given the rarity of these specific defects in the current high-quality state of Kotlin. Therefore, given that readability is the main driving force behind productivity, sacrificing readability would be going against one of the main principles of Kotlin to be pragmatic in helping developers get work done quicker. New language features that target defect rates should do so without sacrificing readability. Reducing readability also negatively affects defect rates as that increases the chance of making incorrect assumptions. Since this turned into a book (sorry), I'll come back when I have more time and analyze this feature along with minor tweaks that should make it more closely aligned with the main philosophy of Kotlin. |
I agree, this discussion feels incomplete. There was some back-and-forth on the what and how, but I'm still missing the why. What is the purpose of the proposed change?
I've always appreciated the rigorous discussion and lively debate that precedes any changes in Kotlin, even for standard library functions. New features mean more complexity, and so a change needs to bring a lot to the table in order to outweigh that cost and justify its inclusion. Language design changes need to meet an extraordinarily high bar, proportionate to the scale of their impact. What problem is this proposal trying to solve, and why does that problem justify a change to an established programming language? |
+1 on the fact that this features doesn't meet the minus 100 points rule IMHO, as the inherent complexity of adding a new language construct is not justified by a large amount of use cases that would actually greatly benefit from this feature, or lack of valid alternatives (as previously mentioned in this thread). Starting from the top, the first comment that comes to my mind when I see such piece of code in a review is "Can we refactor it?" From fun render(status: Status): String = when (status) {
Status.Loading -> "loading"
is Status.Ok if status.info.isEmpty() -> "no data"
is Status.Ok -> status.info.joinToString()
is Status.Error if status.problem == Problem.CONNECTION ->
"problems with connection"
is Status.Error if status.problem == Problem.AUTHENTICATION ->
"could not be authenticated"
else -> "unknown problem"
} to fun render(status: Status): String = when (status) {
Status.Loading -> "loading"
is Status.Ok -> if status.info.isEmpty() -> "no data" else status.info.joinToString()
is Status.Error -> when(status.problem) {
Problem.CONNECTION -> problems with connection"
Problem.AUTHENTICATION -> "could not be authenticated"
else -> "unknown problem"
}
} which is much clearer IMHO (or even moving the nested when to an Why would we prefer to have a flat view that mixes conditions, checking multiple variables, over a structured approach that follows more closely how a programmer reasons? Indeed in the example below it becomes harder for me to understand when "problem try again" is the result: fun render(status: Status): String = when (status) {
Status.Loading -> "loading"
is Status.Ok if status.info.isNotEmpty() -> status.info.joinToString()
is Status.Error if status.isCritical -> "critical problem"
else -> "problem, try again"
} Here I have to skim through all the mixed conditions in all the branches to understand when "problem, try again" can happen. While in the "classical" example I can simply traverse the tree of conditions that brings me to the "problem, try again" leaves, which is simpler to do. fun render(status: Status): String = when (status) {
Status.Loading -> "loading"
is Status.Ok ->
if (status.info.isNotEmpty()) status.info.joinToString()
else "problem, try again"
is Status.Error ->
if (status.isCritical) "critical problem"
else "problem, try again"
} The fact that there is duplication in this approach is not harmful IMHO as it clearly indicates that we return the same result in two different branches of this exhaustive decision tree, compare to the proposed guards solution, which is a non-exhaustive tree + catch-them-all Then the following example shows how the limitations in guards actually end up making you write code that goes against the reasons why guards are added in the first place anyway: fun testString(response: String?) {
when (response) {
null -> { }
"y", "Y" -> println("You got it")
"n", "N" -> println("Shame")
else if response.equals("YES", ignoreCase = true) ->
println("You got it")
else if response.equals("NO", ignoreCase = true) ->
println("Shame")
else -> println("Sorry")
}
} Here we violate both the
If this code was in review, I would suggest to have fun testString(response: String?) {
when {
response == null -> Unit
response.isYes() -> println("You got it")
response.isNo() -> println("Shame")
else -> println("Sorry")
}
}
// Or maybe we could work on this feature instead to remove the duplication, for example
/*
fun testString(response: String?) {
when(response) {
null -> Unit
.isYes() -> println("You got it")
.isNo() -> println("Shame")
else -> println("Sorry")
}
}
*/ Anyway, guards are not really the best option in this case as well. Continuing,
I think this is not a good replacement for comparisons with other languages. Which other languages have guards as in the proposed Kotlin version? What are the differences? Why are we mentioning full pattern matching and how do the proposed guards work similarly/replace it? Why would we want guards to cover the full pattern matching use cases with this solution, instead of adding full patter matching?
I don't fully get this point. Can we get a real world use case? Also, from reading this, generally we want to perform the side effect calls only once, suggesting that the deduplicated (without guards) solution would be the preferred one. |
I absolutely agree with all recent comments. Unfortunately, I never found time to actually discuss it, and I am very glad that the community returned to it. I agree that KEEP was rushed and is not convincing, at least with current examples. I would like to use |
A truly useful guards feature would be able to make the below function actually look like Kotlin. Otherwise, this feature simply has no use case. @OptIn(ExperimentalContracts::class)
@ExperimentalSerializationApi
private inline fun <T> determineEffective(
original: T,
descriptor: SerialDescriptor,
inheritWrapping: (T) -> T,
wrapper: Wrapper<T>
): T {
contract {
callsInPlace(inheritWrapping, InvocationKind.AT_MOST_ONCE)
callsInPlace(wrapper)
}
when (descriptor.annotations.singleOrNull<NonLinkedJson>()?.wrapWithValueObject) {
true -> return primitive(wrapper, original)
false -> return original
null -> { /* do nothing */ }
}
if (descriptor.annotations.containsAny<NonLinkedJson>()) return primitive(wrapper, original)
var effectiveDeserializer = original // also, get rid of the var. Mutability in Kotlin, fr?
if (descriptor.kind is StructureKind.LIST) {
if (descriptor.annotations.containsAny<Container>()) effectiveDeserializer = wrapper.invoke(
effectiveDeserializer,
"quest.laxla.khuzdul.Container",
JsonLdList,
StructureKind.CLASS
)
} else {
if (descriptor.kind is PrimitiveKind) effectiveDeserializer = primitive(wrapper, effectiveDeserializer)
effectiveDeserializer = wrapper.invoke(
effectiveDeserializer,
"quest.laxla.khuzdul.Functional",
JsonLdValue,
StructureKind.LIST
)
}
return inheritWrapping(effectiveDeserializer)
}
private inline fun <T> primitive(wrap: Wrapper<T>, value: T) = wrap(
value,
"quest.laxla.khuzdul.Primitive",
JsonLdValue,
StructureKind.CLASS
) Edit: it is possible in Kotlin today. My point worked better than I thought it would. Took an hour to write, but is incredibly faster to read than first version. @OptIn(ExperimentalContracts::class)
@ExperimentalSerializationApi
private inline fun <T> determineEffective(
original: T,
descriptor: SerialDescriptor,
inheritWrapping: (T) -> T,
wrapper: Wrapper<T>
): T {
contract {
callsInPlace(inheritWrapping, InvocationKind.AT_MOST_ONCE)
callsInPlace(wrapper)
}
return when (descriptor.annotations.singleOrNull<NonLinkedJson>()?.wrapWithValueObject) {
true -> primitive(wrapper, original)
false -> original
null -> inheritWrapping(
if (descriptor.kind !is StructureKind.LIST) wrapper.invoke(
if (descriptor.kind is PrimitiveKind) primitive(
wrapper,
original
) else original, // wrapped original 2
"quest.laxla.khuzdul.Functional",
JsonLdValue,
StructureKind.LIST
) else if (descriptor.annotations.containsAny<Container>()) wrapper.invoke(
original,
"quest.laxla.khuzdul.Container",
JsonLdList,
StructureKind.CLASS
) else original // wrapped original 1
)
}
}
private inline fun <T> primitive(wrap: Wrapper<T>, value: T) = wrap(
value,
"quest.laxla.khuzdul.Primitive",
JsonLdValue,
StructureKind.CLASS
)
|
The purpose of guards and part of pattern matching is to flatten. Let's reconsider the sealed class Status {
Loading,
Error(val problem: Problem, val isCritical: Boolean),
Ok(val info: List<String>);
}
enum class Problem { Connection, Authentication, Unknown }
fun render(status: Status): String {
return when (status) {
Loading -> "loading"
is Ok { val info } -> if (info.isEmpty()) "no data" else info.joinToString()
is Error { val problem } -> when (problem) {
Connection -> "problems with connection"
Authentication -> "problems with authentication"
else -> "unknown problem"
}
}
} Have you noticed that, in nowadays' Kotlin there's no way to express these structured cases in a more straight way? We have to nest over and over again to match the situations, and this is an anti-pattern of data-oriented programming, which we've been striving for back when we introduced data classes and sealed hierarchy. fun render(status: Status): String {
return when (status) {
Loading -> "loading"
is Ok { val info } if info.isEmpty() -> "no data"
is Ok { val info } -> info.joinToString()
is Error { problem = Connection } -> "problems with connection"
is Error { problem = Authentication } -> "problems with authentication"
is Error { problem = Unknown } -> "unknown problem"
}
} The difference is now obvious (at least more than what the proposal says). We can see how these compound cases are matched against, one for each line and each line gives us a value, without having to nest all the way into the deepest structure. Even if we don't cover all the cases like this: fun render(status: Status): String {
return when (status) {
Loading -> "loading"
is Ok { val info } if info.isNotEmpty() -> info.joinToString()
is Error { val isCritical } if isCritical -> "critical problem"
else -> "problem, try again"
}
} We still keep this "checklist" style, with a huge visual improvement from traditional fun render(status: Status): String {
return if (status is Loading) {
"loading"
} else if (status is Ok { val info } && info.isNotEmpty()) {
info.joinToString()
} else if (status is Error { val isCritical } && isCritical) {
"critical problem"
} else {
"problem, try again"
}
} The mental burden caused by not being exhausted is yet another question - should we enforce exhaustion? Well, how about we do that: fun render(status: Status): String {
return when (status) {
Loading -> "loading"
is Ok { val info } if info.isNotEmpty() -> info.joinToString()
is Ok -> "problem, try again"
is Error { val isCritical } if isCritical -> "critical problem"
is Error -> "problem, try again"
}
} If I were to exhaust the cases, is it clear enough? And look, everything is still aligned perfectly. We don't have jagged cases anywhere. How about we rewrite the example above me? SerialDescriptor { val annotations, val kind } = descriptor
val wrapWithValueObject = annotations.singleOrNull<NonLinkedJson>()?.wrapWithValueObject
val hasNonLinkedJson = annotations.containsAny<NonLinkedJson>()
when {
wrapWithValueObject == true -> return primitive(wrapper, original)
wrapWithValueObject == false -> return original
hasNonLinkedJson -> return primitive(wrapper, original)
}
val effectiveDeserializer = when (kind) {
StructureKind.List if annotations.containsAny<Container>() -> wrapper.invoke(
original,
"quest.laxla.khuzdul.Container",
JsonLdList,
StructureKind.CLASS
)
StructureKind.List -> original
is PrimitiveKind -> wrapper.invoke(
primitive(wrapper, original),
"quest.laxla.khuzdul.Functional",
JsonLdValue,
StructureKind.LIST
)
else -> wrapper.invoke(
original,
"quest.laxla.khuzdul.Functional",
JsonLdValue,
StructureKind.LIST
)
}
return inheritWrapping(effectiveDeserializer) |
@serras , I finally got a chance to get back to this so I'll focus on the readability problems and mental model issues and provide suggestions to address them. As a brief refresher, my previous comment showed that reducing readability effectively goes against one of Kotlin's main principles of being pragmatic as readability is by far the largest contributor to developer productivity due to the 10:1 ratio of reading to writing code. Reducing readability also increases defect rates. Readability TestTo test whether a change is readable, developers who haven't heard of this feature should:
I should note that any changes that break the existing mental model automatically reduce readability and thereby productivity. Lastly, we should think twice before trying to replicate the syntax of languages that have a steeper learning curve than Kotlin (like Scala, Rust, etc.) as those would usually take Kotlin into a less-readable direction. Readability Problems1. Breaks the mental model of wrapping if-conditions in parenthesisThe KEEP allows trivial lines like this in a
This is a trivial example so typical conditions are even more complex. The easy fix to this problem is to align with the existing mental model for if-statements and always require parenthesis around the condition:
This rule should always apply even if the condition is based on a single boolean variable. Readability is much more important than saving 2 keystrokes. 2. Breaks the mental model that
|
As a Rust enjoyer myself, I can say that guards in match/when patterns have, imo, 0 impact on those points. Guards are just a condition on whether that branch is going to be executed if the pattern does indeed match. Would that help if you knew from the start that |
Saying that the changes in the KEEP have 0 impact on those points makes me think that personal preferences are at play rather than looking at it objectively. Or it could be one of those things where once you've gone through the effort of internalizing a concept, you forget about the effort it took to get there. For example, I met an expert skier that told me skiing is as easy as riding a bicycle so there's nothing to it. While an expert skier might feel like there is nothing to it, the truth is that they need to be much more alert and responsive while skiing compared to riding a bicycle. You might feel very comfortable in Rust with guards but that doesn't mean there isn't an easier and more intuitive way in which the language can add guard conditions. I'm not intending to start a language war here as Rust would be my first choice for systems programming. However, while the productivity of this KEEP might seem on par with guards in Rust, Kotlin is a more productive language than Rust so matching Rust would effectively reduce the productivity of Kotlin. I would love to see full pattern-matching support in Kotlin. I also see some value in guard conditions in the current state of Kotlin so I'm not opposed to the concept. My main concern is that the KEEP in its current form introduces too many sacrifices which break the existing mental model and significantly degrade readability effectively reducing productivity. My previous comment provided suggestions that would address my readability concerns. While Kotlin is significantly more readable than Java in general, the KEEP in its current form makes guard conditions less readable than guards in Java: I would like to see this KEEP enhanced to meet minimum readability standards based on the current Kotlin mental model and then continue to the next step of the language enhancement process such as ensuring soundness etc. |
Would that help if I noted that Java/C# uses |
Because as mentioned above, Kotlin is a more productive language than Rust, Java, Swift, etc., so it is held to a higher standard. Plus, Kotlin is a different language with different design choices. Your argument is Ad Populum; the fact that others do it doesn't make it good generally, nor does it make it good for Kotlin specifically. If you're saying Rust designers know what they're doing, that's Ad Verecundiam; everyone can make mistakes. So please, let's ignore what others do here, and talk about what Kotlin should do. |
As I said, guards and pattern matching are essential to data oriented programming. Without guards, we can only nest; without pattern matching, we can only nest. Please take a look at my comment above, the ability of easily composing conditions does make code more readable. We can choose another keyword like when (status) {
is Ok -> when {
status.info.isEmpty() -> "No data"
else -> status.info.joinToString()
}
is Err -> when (status.problem) {
Connection -> "Connection fails"
Authentication -> when {
status.level > Level.Fatal -> "Fatal error: Authentication fails"
else -> "Authentication fails"
}
else -> "Unknown problem"
}
}
when (status) {
is Ok { val info } if (info.isEmpty()) -> "No data"
is Ok { val info } -> info.joinToString()
is Err { problem = Connection } -> "Connection fails"
is Err { problem = Authentication, val level } if (level > Level.Fatal) -> "Fatal error: Authentication fails"
is Err { problem = Authentication } -> "Authentication fails"
is Err -> "Unknown problem"
} |
Pattern-matching covers a few different things. In its most basic form, like Java's When pattern matching was extended to When guards were added to But in Kotlin, things are very different. With any luck, we'll soon have exhaustiveness checks everywhere. If we want exhaustiveness plus extra branching logic, we can do it by adding exhaustiveness to our existing conditionals, instead of adding new conditionals to our existing exhaustiveness! That's awesome, because it means that—unlike Java—we're not going to be forced into adding new syntax. We need to build on Kotlin's existing concepts and features, and solve the problems we actually have, not the ones we see people solving in other languages. Guards are designed to solve a problem that does not exist in Kotlin. Smart casting is not the same as pattern matching, and Kotlin is not the same as Java. |
There is a problem with current Kotlin - we can't early return in |
I agree about the limitations of |
@Peanuuutz The examples you bring is a proposal for full pattern matching syntax (which is not yet in the language and not part of this keep). With guards only, the code will look much different: when (status) {
is Ok -> when {
status.info.isEmpty() -> "No data"
else -> status.info.joinToString()
}
is Err -> when (status.problem) {
Connection -> "Connection fails"
Authentication -> when {
status.level > Level.Fatal -> "Fatal error: Authentication fails"
else -> "Authentication fails"
}
else -> "Unknown problem"
}
}
// With guards
when (status) {
is Ok -> if status.info.isEmpty() -> "No data"
is Ok -> status.info.joinToString()
is Err if status.problem == Connection -> "Connection fails"
is Err if status.problem == Authentication && status.level > Level.Fatal -> "Fatal error: Authentication fails"
is Err if status.problem == Authentication -> "Authentication fails"
is Err -> "Unknown problem"
}
/* Full pattern matching. Not a thing (yet?)
when (status) {
is Ok { val info } if (info.isEmpty()) -> "No data"
is Ok { val info } -> info.joinToString()
is Err { problem = Connection } -> "Connection fails"
is Err { problem = Authentication, val level } if (level > Level.Fatal) -> "Fatal error: Authentication fails"
is Err { problem = Authentication } -> "Authentication fails"
is Err -> "Unknown problem"
}
*/ Indeed, I don't get why we want to add guards before adding pattern matching, it will just cause the language to temporarily push/allow people to use the second version of the example above, and when (hopefully) we will have a better pattern matching syntax, we should switch again to the third version, causing the code written with the second version to become non-idiomatic, and have two new learning curves instead of one for the same goal. Most likely having guards with this syntax will also become a limitation of the possible syntaxes used for pattern matching. See comment here (not mine but I agree). If you want to use the guards version, you can do it already in kotlin with minor differences:when {
status is Ok && if (status.info.isEmpty()) -> "No data"
status is Ok -> status.info.joinToString()
status is Err && status.problem == Connection -> "Connection fails"
status is Err && status.problem == Authentication && status.level > Level.Fatal -> "Fatal error: Authentication fails"
status is Err && status.problem == Authentication -> "Authentication fails"
status is Err -> "Unknown problem"
} which looks much more similar to the guards version than the "patter matching/destructured one", and even looks more coherent since The main focus of the discussion for me here is not about the fact that we need something more for data oriented programming (youtrack is the place for that kind of discussion), but about the proposed solution and the impact it has on the language. Guards proposed like in the merged KEEP do not replace pattern matching nicely (like in other modern languages) at all, and do not solve the exhaustiveness issue (for which the proposed solution suggests using the current standard and nesting), so when used with ADTs the proposed solution reduces the inherent safety of the language. This is how I would write it if I really wanted to avoid nesting (which would probably not be the case for this use case), even with guards available in the language: when (status) {
is Ok -> if (status.info.isEmpty()) "No data" else status.info.joinToString()
is Err -> status.explained
}
val Err.explained: String get() = when (this) {
Connection -> "Connection fails"
Authentication -> if (level > Level.Fatal) "Fatal error: Authentication fails" else "Authentication fails"
Storage,
OtherError -> "Unknown problem"
} I'm not saying there is no room for improvement or that nesting is the only way forward, but just that the proposed solution has many drawbacks, doesn't add much power of expressiveness to the language, and should be reconsidered IMHO when thinking about the design of complex patter matching. |
Actually I'm also more fond of pushing off guards and waiting for full pattern matching. I'm here only to elaborate that guards do serve a purpose and are not redundant in the evolution of Kotlin language. We can make guards be experimental until that point. I'm just scared to see it being completely dropped. |
Let me provide a bit more rationale and motivation behind our decisions. Guards and parenthesisThis is something we argued about in the early stages of KEEP. Note how this problem arises only because More importantly, we see that guards are mostly used for simple checks like But why did we pick “if”?First of all, we can't use any new keyword like Next, if we examine the use of potential Lastly, There was an argument about the combination of It might lead to the impression that everything is decided and there is no room for change. Please know that this is not the case. We shared this KEEP three months before KotlinConf and one of the intentions was to discuss new upcoming features in person, which was great! Does it mean we rushed with decisions? Oh, no, the idea of introducing guards as an incremental improvement for pattern matching facilities appeared long time ago, but now we finally write our conversations down and ready to move with it further. The feature is going to be shipped in its first iteration and we’re preparing a bunch of activities to validate some of our assumptions. One of them is about the necessity of parentheses and “feature abuse”, and this is why we release features in Betas and enable them for our internal projects, to actually see how people use new features. We conducted a research before KEEP and we’re going to do it again after actual on-hands experience. So, thanks for your concerns. That’s something we’re going to recheck with more data! Finally, I see a lot of comments thinking that just because we add guards, this is the end of the game for more data-oriented programming features. It is not. We will provide a more detailed response to address comments about pattern matching and to highlight Kotlin-specific variables involved in this design equation. We plan to show how these variables can be integrated into Kotlin's evolution, ensuring that guards is just one part of a broader strategy for enhancing data-oriented programming features. |
@zarechenskiy thanks for the follow-up. It's great that this is going through another cycle of feedback based on actual uses of the feature, however, I wonder if it shouldn't be released in a prototype/experimental/alpha stage first. Beta usually means "We are committed to the design, but it still needs bug fixes and optimizations". What's the reasoning for jumping straight to Beta in this case? |
@edrd-f Valid point! I used the word 'iteration' in my answer for a reason: terms like alpha and beta vaguely describe the status of features. We’re going to shift to 'iterations' to avoid the assumption that a feature in Beta means it will be released in the next version no matter what. We will provide proper IDE and compiler support for features in any iteration, but we also want to give our language the time it needs |
@zarechenskiy So does that mean that Beta means "proper IDE and compiler support" while alpha/prototype/etc means "some compiler support, minimal IDE support" or something along those lines? |
@kyay10 Right |
I may be missing some important use cases, but I still don't see any good example on Keep, where guards are improved code. Examples from KEEP about error messages and Yes/No parsing are already questionable enough and many examples in this thread show that without this feature they could be rewritten at least not worse, but arguably better than the proposed Guard solution Mihail mentioned it as a partial solution. Maybe I don't see the whole picture, but I agree with some of the commenters that in the current state, it doesn't follow the -100 points principle, creates hard-to-read code, and even promotes a bad style of programming (so it goes against what is considered good style with exhaustive checks as much as possible now). If it is just a step, and potential improvements in the future require this, I can see it, but I just don't see that we ever allowed to use this style in our code base, maybe I just missing examples where I would say "yes, now it's more clear/readable" Considering that this feature will be added as experimental any case, I think we should just wait and see, hope that we will get a new KEEP to discuss evolution and feedaback |
Alternately to my suggestion of adding support for |
We've described above some of the reasons for this choice. I don't think anything has changed since then. Once the feature is released (it should be in 2.1) we'll gather information about their usage, and decide whether |
Frankly, it appears to me that |
This is an issue to discuss guards. The current full text of the proposal can be found here.
This KEEP proposes an extension of branches in
when
expressions with subject, which unlock the ability to perform multiple checks in the condition of the branch.The text was updated successfully, but these errors were encountered: