Best practices to send a throwable from Fragment/Activity?

mochadwi commented 3 years ago

MVFlow class

    sealed class Error : Action {
       object MaxChar : Error()
       object NPE : Error()
       object UPA : Error()
    }

In Activity/Fragment

private val actionsChannel = Channel<Action>()

// somewhere in the code
// e.g: 1
viewBinding.editText.doAfterTextChanged {
   if (it.length > 200) actionsChannel.offer(Action.Error.MaxChar)
}

// e.g: 2

try { 
    doSomething(null)
} catch (e: NullPointerException) {
    actionsChannel.offer(Action.Error.NPE)
}

fun doSomething(nonNull: String) {
 // do something
} 

// e.g: 3

private var lateinit abc: String

// we forgot to initialized,
// therefore will throw UninitializedPropertyAccessException: lateinit property `abc`
try { 
   doSomething(abc) 
} catch (e: UninitializedPropertyAccessException) {
   actionsChannel.offer(Action.Error.UPA)
}

Question

or is there any alternative to distinguish between throwing errors in our view (e.g: without an action), maybe using Effects #30?

pedroql commented 3 years ago

Hi,

To be clear, the question you are asking is not specific to this library. It's a general question about good practices applied to MVI (but is not even MVI-specific).

So everything I am saying here is my opinion and you and others are welcome to make your own opinion. The library doesn't prevent you from doing any of that.

Looking at your first example, that looks fine. To be clear, the user typing more than some number of characters, is not an "exception". It is something some users will do and there's nothing wrong with that. Your app just needs to respond in some way. So that example looks good IMO. [But you probably need to do something to clear the error state once the user deletes some characters].

Your second is a bit more difficult to give a clear-cut answer because some of my comments might be coming from you giving a small/simplified example. Most of the times you can avoid the exception from being thrown by checking for null before making a call (or using kotlin syntax). Some exceptions are outside your control, for example, intermittent network failures. Once again, these are not exceptions in the sense that these are things that happen in normal day to day life and your app needs to handle them accordingly. Depending on what networking libraries you use, you might receive these events as exceptions (and have no alternative other than catching an exception) or you might have better (cleaner APIs) to deal with those scenarios. Either way, at some point you realise that something didn't go to plan and need to update the UI. Sending an action for this is fine. In your example, you called this Action.Error.NPE. Personally, I think the business logic doesn't care that it was a NullPointerException (and fair enough, you are not passing the java Exception instance) but I would probably call it something else. Anyway, this is just semantics. The code you wrote would do exactly the same thing but to a human code reviewer it might feel a better way to model what is going on.

About the third example has the same general message as the previous two paragraphs but this particular case it doesn't seem that we are talking about something that should happen. It seems to be that it is a programmer error and that given enough time you can make sure it never happens. So IMO the bug should be fixed and you don't need to wonder how to do MVI with this. If you tell me you can't fix it, I don't know what you should do because I would need to speculate about what your app is doing and how you will handle this exception... And that's too speculative.

While this is not mentioned directly in any of the questions/answers, in general, if one exception is thrown in your activity, your handler/reducer/view don't care about java exceptions, they care about things with a higher-level meaning such as "Invalid input" "No connectivity", etc. So if one exception occurs you want to translate it to something with more meaning than a general java class that comes bundled in the SDK and this is what your handler/reducer/view care about. This is what you do well in the first example.

Understanding "best practices" is not easy and it comes with time and mistakes... and having to live with them until you refactor them. And it's also subjective so there is not one single opinion that is right/wrong.

mochadwi commented 3 years ago

I see, really appreciate it for your brief explanation 🙏

absolutely agreed, we're trying to gather information regarding "catch an exception" from activity/fragment and we're assuming every throwable occurred we can "catch them all" and translate it as an action.

in other hands, opening this issue, you might have suggestion or maybe an approach to handle such cases

Then using Effects is not suits this particular usecase ya? and the approach no. 1 is preferred, but we need to extra careful to reset the state (similar to #30) after user is inside the char threshold

pedroql / mvflow