-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Flow.firstOrNull should allow nullable types #2229
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
Comments
It is indeed not allowed to properly differentiate between empty flow and flow with We are open to revisiting this decision, but at least requires a compelling use-case apart from technical reasons |
I feel like being able to differentiate between an empty flow and a null value is a weaker use case then simply wanting the first element of a flow with a nullable type. My use case is that I often have flows with a nullable type where I want to await the first element. For my purposes I don’t care if the item is null or the flow is empty. You’ll still be able to differentiate between empty flows and null when using non nullable types and for nullable types you’ll simply have to use |
@ansman Can you give a more specific example of the kind of flows of nullable items that you work with, please? What are those? What is the purpose of nulls in your case? |
One such use case is reading a value from disk. I have a "settings" class that stores keys and values. Observing a key returns a flow with a nullable type since the key might not exist. If there is catastrophic error while reading the settings I simply close the flow rather than throw an exception since for consumers it doesn't matter why, they simply care that there is no key. I could of course just emit I feel like wanting the first or null from a flow is a real use case while differentiating between |
I strongly disagree that exceptions should be used to detect the empty case. In general, exceptions should not be used for control flow at all, they are expensive and should be used to represent only "exceptional" cases, as the name suggests (e.g. incorrect usage of an API). I also disagree that using null to detect empty is purely hypothetical. I have also written lots of code (albeit with lists, not flows) that uses Your example has some pretty specific concerns, eg that emitting a null at the wrong time could overwrite a previous value. That seems very specific to the use case of a data repository type of API. Also, your reason for not taking the simple approach of emitting null before closing is definitely hypothetical (somebody could one day change the code), and I don't think it's a good justification for not taking that approach: of course someone could change any code some day and break anything - code isn't written in stone. One of the jobs of unit tests is to verify important, load-bearing behavior like that. I think it makes sense for the stdlib to provide functions that err on the side of caution and safety and don't expose ambiguities. Given the current stdlib function, it might not cover every possible use case but its returning null has a single meaning and it means the same thing in every codebase. You can easily write your own extension function that does what you want and keep the ambiguity scoped to your codebase, where you're willing to accept the potential risk that comes with the ambiguity. However if the stdlib were to include the function you're requesting, that ambiguity would be introduced into every kotlin codebase, and it would be much harder to just look at unfamiliar code and figure out if the null value is being used for both semantics, or was intended to only mean one thing and there's a hidden bug. This is one of the major issues with Java APIs that Kotlin solves, it would be a real shame to go backwards. |
Firstly I doubt an exception will make any impact on performance in this case. Secondly You can still write code like that, if you are using non null items. Same as today. My point is that if you need to check if the flow was empty when using nullable types you may use a try-catch to do so. I think the difference between list's and flow's version is strange and currently there is no way to get the first element for a flow with a nullable type. Making an extension is always possible but naming will be difficult and it will take more cognitive load for the reader to understand why Making this change will not affect any existing code or new code that uses flows with non null types. |
* Allow nullable types in Flow.firstOrNull * Allow nullable types in Flow.singleOrNull * Align Flow.single and Flow.singleOrNull with Kotlin standard library Fixes Kotlin#2229 Fixes Kotlin#2289 Co-authored-by: Nicklas Ansman Giertz <[email protected]>
* Allow nullable types in Flow.firstOrNull * Allow nullable types in Flow.singleOrNull * Align Flow.single and Flow.singleOrNull with Kotlin standard library Fixes Kotlin#2229 Fixes Kotlin#2289 Co-authored-by: Nicklas Ansman Giertz <[email protected]>
Flow.firstOrNull
does not allow nullable types, presumably so that you can differentiate between the flow being empty or not. However, every otherfirstOrNull
function such asList.firstOrNull
does allow nullable types and I see no technical reason why the flow version should not allow nullable types.This change would be binary compatible since the type argument's nullability isn't even reflected in the bytecode. It would also be source compatible since it only loosens the restrictions.
The text was updated successfully, but these errors were encountered: