https://kotlinlang.org logo
Join Slack
Powered by
Hello guys, What is your opinions about putting al...
# android-architecture
a
Hello guys, What is your opinions about putting all the relevant ui-events in the uistate and making all the methods in viewmodel private and bake the uistate with that methods references? data class UiState( val name:String val setName: (String)->Unit ) That setName method will be filled with the method in the viewmodel
p
How do you provide this 
UiState
to your Composables?
a
From the ViewModel nothing special then what we do normally. val uiState = _uiState.asStateFlow()
p
Then I'm not sure it's going to work, if you call 
setName()
on 
UiState
, it won't trigger an update and recomposition. You need to set the whole state using 
copy(name = "name")
I think.
a
the setName method wtill me something like this: private fun setName(value: String) { _uiState.update { copy(name = value) } }
image.png
p
I see, then it should work but I don't think it's a good approach, way too much boilerplate than just calling copy, especially if you will make set methods for each or most of the fields in state.
a
In that usage i define a master uiState which contains smaller composable states in that state with each state object contains only the valid methods for that state so composableA never be able to call a method for composableB from the viewmodel
and i pass a single state object to composables
on that particular screenshot for examlpe i display a bottomsheet when the airportSearchUi state filled so i also dont need to handle another flow to pass events to composable
And you should give some kind a setName method anyway from somwhere
p
setName
method on 
UiState
? You don't need it. If anything, argument can be made it's wrong because it violates immutability of Kotlin data class.
"That setName method will be filled with the method in the viewmodel" Passing a method from viewmodel to uistate just doesn't make sense to me.
a
Thats not violates immutability as long as u annotate it as immubatle
g
Every team I've been in decided collectively to not do that.
a
Have they ever tried? And what is the reason behind that? Im asking cause i tried and i didnt see any problem so maybe im missing something or its just people used to do things in one direction
When i think about state, the data it holds and the acitions that modifies that data needs to sit together.
Also the sateholder classess in compose kind a smilar things imo
g
@Ahmet Özcan It's about • Mixing actions and states • You're kind of leaking the VM to the UI via the UiState, and it should just be a state, now it's an ui bridge • Possible recomposition issues that could arise from having lambdas in there • No way to control or enforce a certain pattern (like an MVIViewModel) that helps teams think the same way
💯 3
But to each their own, if it works for you and you love it, nobody can force you to not do that, but that's the general consencus around the subject 😊
p
I have not seen any other usage (excluding few exceptions) than updating state as something like 
state = state.copy(value = newValue)
via VM methods. Overall State object in compose should not have any setters (again, coming back to data classes being immutable). It doesn't randomly change properties of state, it's simple and very easy to test. I feel like this is generally used approach.
a
There is no usage like sate = state.copy in my example but nvm. I just wonder how can we do things in different ways then we always do. Thank you very much for your responses. K
p
I know it's not there, that's the point 😄 As Alex said, to each their own
🙏 1
s
am i missing something, data classes should be data aka states? i would evaluate, does it make sense for a data class to contain action, despite marking classes & types with 
@Immutable
? it seems to fundamentally break the concept of data classes. also, does "do things in different ways then we always do" justify breaking fundamental concepts? thinking about it in other directions, can the lambda also return a non-
Unit
type, i.e. is this acceptable: 
val returnAction: () -> String
could the lambda be of type generic: 
val genAction: T.() -> Unit
or 
val genAction: (T) -> Unit
a
It's not containing the action just contains reference to an action actually.
If you think about stateholder classes they contains both state and the action
Or viewmodel its self contains the state and the action if you look that way
p
But viewmodel itself doesn't represent the state, it's a coupling mechanism, state should not contain actions. ViewModel provides access to state via methods.
Separation of data and logic is a fundamental rule in computer science.
👍 1
a
The stament ur giving is wrong. Data class that im talking about is not contains any action. Its just contains the ref of the action. They are very different.
Referance itself also a data
Im not talking about putting any logic into the data class
p
Data class you're talking about contains a reference to an action which you're passing from VM if I understood correctly and later calling it by referencing the state (eg 
state.setName(nmae)
). I can paint a tomato green but it still will be a tomato. That's another thing btw, why do you think it's better to pass function reference to state instead of calling VM methods directly in composables?
a
I dont think its better btw 😄
Imagine that you have a screen that displays an otp input and it counts down from 30 lets say
At the 15th second u would like to show an resend button
Normally you can call the resend action in any time if you want to access that from viewmodel
If you change the state to sometginh else at the 15th second and give the resend ref to state and keep the actual method private you cant call it when ever you want but should wait for the proper state
When you update the state at 15second to something meaningful for ui it could display the resend button and access to proper action throught the state resend method ref it needs
Ofc you can do this in million different ways
p
Yes but you don't need to reference some action for it, you can do it like this, it's much easier, less messy, much easier to test...
Copy code
data class State(
    val remainingTime: Int = 30
) {
    val isResendButtonVisible: Boolean
        get() = remainingTime < 15
}
Copy code
class TimeViewModel : ViewModel() {

    private fun countdown(duration: Duration): Flow<Duration> = flow {
        var timeLeft = duration
        while (timeLeft > 0.seconds) {
            emit(timeLeft)
            timeLeft -= 1.seconds
            delay(1000)
        }
        emit(0.seconds)
    }

    fun startCountdown() {
        viewModelScope.launch {
            countdown(30.seconds).collect { timeLeft ->
                state = state.copy(remainingTime = timeLeft)
            }
        }
    }

    data class State(
        val remainingTime: Int = 30
    ) {
        val isResendButtonVisible: Boolean
            get() = remainingTime < 15
    }
}
(this will not compile btw, just wanted to show a prototype)
a
Yes thats ok but what i want to secure is the method for resend action. Since the redend method is public i can call it at firsr second and nothing can stop me doing that
p
Then you just check what 
remainingTime
in state is
a
I just want to say it feels more natural that i can take the actions that the state im in noting more nothing less
p
I understand
a
Slack's circut as i know access the action throught state
That's not something new but feels like neee more attention
p
Do you mean the concept 
eventSink
in Circuit?
a
Yes
p
Yes, I found it I hate it 😄 No I don't hate it, it just feels counterintuitive to me
a
yes that's different then what i tried to say 😄
but smilar
insted i dont keep any logic in state class just brings the correct action refs
p
I used to see a lot this way of implementation
Copy code
fun onEvent(event: Event) {
    when (event) {
        ...
    }
}

sealed class Event {
    ...
}
but the 
onEvent
function was part of VM. I used to like it but today I think it's not as readable as updating State in smaller functions
a
im agree on that
Give it a try the concept of action refs ☺️
on the testing side nothing very different
j
Lambdas are not serializable, so you can't restore the state class using bundles...
a
And you dont need to serialize methods. You just add the data you need to restore in the saved state and bake the methods again when restored
j
That is easier said than done,it is doable, but more work too
a
You can just mark the fields that you dont want to be serialized i guess
j
No, you can't. You need to set up the lambdas again when the is created again.
a
Yes but u will be doing that when you givd the initialState to flow anyway
👍 1
s
It’s not good to put behaviors like callbacks in the UIState data classes only for data and if you need to include functions or behaviors like callbacks, you should use a regular Class, by using a regular class, you clarify intent, avoid unintended behavior from generated methods, and uphold the principle that data classes should only hold data check this blog for more details: https://blog.mmckenna.me/a-curious-case-of-mistaken-identity
👍 1
a
This post is not against it just point to a miss usage case.
Method reference its self - again is a data
👀 1
You should use it excatly how he wrote on the article as a reference with :: notation
s
i don’t think so, he just provided another option in case you need to use those callbacks, but he never suggested putting callbacks inside 
UiState
. functions are not data; they produce values when executed in Kotlin, data classes are meant to hold only data, and the compiler generates key methods for them like (
equals()
and 
hashCode()
) based on the properties in the primary constructor if you include functions as properties, you break the core contract of a 
data class
, and those generated methods will no longer behave correctly. take a look at the last two points he mentioned: • Store lambdas separately from data classes when object equality matters • Override equals and hashCode carefully to exclude fields that could vary unexpectedly, or you know don’t matter to your equality.
a
When you put the actual method reference into the field you no longer need to worry about data equality
Copy code
data class MyData(val name: String, val action: () -> Unit)

fun main() = runBlocking {

    val v1 = MyData("Test", ::test)
    val v2 = MyData("Test", ::test)

    println(v1 == v2)
}


fun test(){}
What do you think that print line will produce ?
in kotlin fuctions are first-class citizens
there is no difference between a primitive variable and a function variable
p
Sounds like "everything is true if you try to justify it hard enough"
a
I'm not trying to justify but if you are coming against an idea you should come with solid proof
p
It's not about proofs, you're basically saying "it works so it's the right approach and you can't say otherwise". Of course it works, that doesn't mean it's better. This "discussion" doesn't lead anywhere.
a
Yep and i never said that this is better. Its just an idea that you can try
p
And so far everybody is telling you "yes it works but it's horrible" 😄
😅 1
👍 1
s
it seams like there's a bit of preference on what a UiState and data classes should declare. the real question is, what problem does having a callback, lambda, function reference, etc. inside a data class or UiState actually solve?
a
And the whole idea behind it to protect events in states so you cannot take any other actions. If you find better aproach then guarding with if / elses be my guest
s
protect events in states so you cannot take any other actions.
how do you mean protect? and how do you mean take any other actions?
a
I mean you should not take any other actions beside the current state that you are in.
s
sorry I'm not following, perhaps I need an example? would an example of "take any other actions" surface the problem?
a
yes ofc give me 5 min in a meeting 😄
👍 1
Thats the previous explanation that i made: Imagine you have a screen that displays an otp input and it counts down from 30 lets say. At the 15th second u would like to show an resend button Normally you can call the resend action in any time if you want to access that from viewmodel. If you change the state to sometging else at the 15th second and give access to resend action to state via method refrence and keep the actual method private in viewmodel you cant call it when ever you want but should wait for the proper state. When you update the state at 15th second to a new state for ui it could display the resend button and access to proper action throught the state resend method ref it needs Sample UiState for accepting events: data class UiState( val text: String, val eventSink: (Event) -> Unit ) sealed interface Event { data class UpdateText(value: String): Event } @Composale fun Form(uiState: UiState) { TextFeild( value = uiState.text, onValueChange = { uiState.eventSink(UpdateText(it)) } ) }
Think that like a very primitve state machine
s
sorry, maybe I wasn't clear. what you've shared above is the solution (lets put a pin on it for now). can you give an example of the problem and maybe elaborate how the problem arises, why the problem is actually a problem, etc.
a
That's not a problem to solve. Its more like a different approach to the viewmodel thing its more intiutive to me to think about the the events that i can take must be limited to the state that im in.
imagine that my body is my state and i have an event named "run". Since that event is lives in viewmodel my body can call that event when ever want. But if some point my state has changes and i have nolonger legs then i should not have an event named run
p
Copy code
fun run() {
    if (!state.body.hasLegs) return
    run.invoke()
}
You don't need events
a
yes its ok but i dont like using guards.
it feels unnatural to me
p
🤷‍♂️
a
That how i done things too in everyday life
using guards but its not ok imo
MVVM is not good enough for very big projects
p
What would be the body of your 
eventSink
method?
a
like in the mvi for example
event it self is a sealed interface
p
Yes, show me the example of code
a
private fun eventSink(event: Event) { when(event) { Run -> run() } }
that eventSink lives in the viewmodel
p
Yes and you're going to pass it to ui state as a reference to function, we know that
😂 1
when (event)... is not a guard for you?
a
its not a guard like business logic
its branch ing for actions
for one sec stop to think about passing events in data class
just tell me if is it really feesl confortable to you to calling any method in the view model when ever you want from the ui 😄
p
Yes, it feels comfortable
And not "any method"
a
if someone removes that guard for some refactoring reason
without knowing why its there
then problems rises
p
I would understand if you had the 
eventSink
public in viewmodel and not pass it to UiState, it's an appraoch, it's ok to pass events if it feels good to you, great but don't tell me you don't like guards when you're checking for all the states in 
when
. No problem arises if you write tests.
I'm done
a
Test is ok but thats a different stroy 😄
Note to the future: One day we ll be using state driven arch. in android development .
s
so if I understood correctly, the problem is that the wrong VM function can be called, correct?
a
yes
feels like no limit here
s
I see, so how does passing the function in a state object solve that?
could the wrong function reference passed to the object also cause the same issue?
p
What if I do this? @Composale fun Form(uiState: UiState) { TextFeild( value = uiState.text, onValueChange = { uiState.eventSink(SignInButtonClickedEvent) } ) }
This is the same like calling the wrong function in your book, you just pass in the wrong event
s
thanks for the example to my question. I'm on my phone 😅
a
Yes it cant stop every possibility but its more structural more type safe
the example you gave is more exaggerated
p
It's not, it's literally the same thing you're saying but with events instead of VM function.
a
in your example you r cllaing the rgiht method with guards
if that guard is not here its a problem
here ur calling something not related to the context
p
You having a 
when (event)
check is also a guard. You still need to check the event to know what to call. Yes, I;m checking conditions in my code, that's absolutely natural thing to do. As I said, passing events is fine imo, if you like it, great, but I really feel like you're trying to point out the weirdest reasons to why what you're proposing is better. 😄
s
like you said, it can't stop every possibility. which sounds like the same problem as wrong function call. whenever either happens it'll fail unit tests and be very obvious when the developers is testing before finishing the task, at least during pre-release testing.
🙌 1
are there any other problems?
p
You think having a state machine is better, great, I think defining bunch of events is an unnecesary hassle and harder to test
a
again idont want to say anthing like its better. I just want to say that the way we r using now is not good noeugh
s
I can see that the possibility of events are also endless, so the level of encapsulation is basically the same as functions
p
That's the same thing, it's not good enough in your opinion so you're coming up with (hopefully) better solutions
s
yes that's fine, that's why I'm asking for the problem. the problem presented so far doesn't seem to be solved
that's why I'm wondering if there are any other problems
a
Yes the problem will still be a problem for a long time just wonder if make it less problematic
at least in way that limiting your possibilities
s
I'm always open to new ideas, even if it breaks principals (e.g. open closed principal is very verbose) but it needs to actually solve a problem
i can imagine the same can be done by just calling that VM function without putting it into a data class object.
🙌 1
the thing is, so far the solution doesn't seem to have an actual problem that it's solving. now, because it's not visible doesn't mean it doesn't exist
a
this is much more organised way to do what im pointing
s
will do
a
and thank you all for contrubuting even some times drives you mad 😄
14 Views
Open in Slack
PreviousNext
Powered by