search

rickclephas / KMP-NativeCoroutines

Library to use Kotlin Coroutines from Swift code in KMP apps
MIT License
1.04k stars 31 forks source link

Add @discardableResult attribute/s #172

Closed thomasflad closed 4 months ago

thomasflad commented 5 months ago

Speaks something against adding the @discardableResult attribute to functions like asyncFunction(for:). Sometimes we just want to ignore the result/return value

vanniktech commented 5 months ago

I'd rather prefer to have the warning, can't you do _ = asyncFunction(for:)? Which is the suggested syntax for this anyway.

rickclephas commented 5 months ago

@thomasflad do you have some examples of cases where you would discard the result? I am not sure if we are going to find a way (other than ignoring all results) that is shorter than _ =, but who knows.

thomasflad commented 5 months ago

Thanks for your comments @vanniktech @rickclephas

We have situations where we don't care about the Result of the suspend function call. Can you explain why do you think it's the suggested syntax to use _ =? We are calling asyncFunction(for:) only for the side effect and not for the result so I think it should be optional to consume the result. Wdyt?

vanniktech commented 5 months ago

Better to be explicit and state with _ = that you don't care whether someone new to the codebase not getting a warning because he didn't know that the return had to be handled.

thomasflad commented 5 months ago

This makes sense when the return has to be handled, but this is not the case in my opinion. When the suspend function is only triggering a side effect and is returning Unit, why is it mandatory to handle the result? When the suspend function is returning a value, then the developer is usually interested in this return value.

vanniktech commented 5 months ago

And by adding @discardableResult you're enforcing everyone to not care.

rickclephas commented 5 months ago

I don't think we should add @discardableResult for all cases. Swift obviously thought about this and decided to show warnings and require developers to be explicit about their intentions.

We might however be able to do something about the Unit return type. That would normally be Void and wouldn't require the result to be handled in Swift.

I'll take a look and see if we can somehow make an exception for the Unit return type.