Hello so I m curious about architecture We are now wrapping

Question

Hello so I m curious about architecture We are now wrapping our UI with something that handles our viewmodel and states and then providing the UI with information rather then passing in the viewmodel Right So starting here how do we handle when there are too many parameters being passed to the UI like 20 In my mind when I read trough this case I would leave it as is because it was pretty much explanatory I also had an option to break it in several smaller UI components but it looked off and ugly like let s say header body and footer How are you handling your viewmodel and UI when your UI needs to receive too many parameters

Cicero · Answer

Example of something I found in our source code ``` exerciseName String exerciseNameHasError Boolean durationTimeState State lt String gt StepDescriptionItems List lt StepDescriptionItem gt bitmapState State lt Bitmap gt onMediaDeleteButtonClick gt Unit onOpenGalleryClick gt Unit onTakePhotoClicked gt Unit onCaptureVideoClick gt Unit onSaveButtonClick gt Unit onExerciseNameChange String gt Unit onStepDescriptionValueChange index Int text String gt Unit onStepDescriptionFocusChange index Int focusState FocusState gt Unit onIncreaseDurationButtonClick gt Unit onDecreaseDurationButtonClick gt Unit onAddNewStepDescriptionClick gt Unit``` with this implementation ``` CreateOrEditExerciseUI exerciseName = exerciseName exerciseNameHasError=exerciseNameHasError durationTimeState = timeFormatted bitmapState = previewBitmapState onMediaDeleteButtonClick = viewModel deleteVideoFile onOpenGalleryClick = viewModel getVideoUri let openGalleryLauncher launch arrayOf video image onTakePhotoClicked = viewModel getPictureUri let takePictureLauncher launch it onCaptureVideoClick = viewModel getVideoUri let captureVideoLauncher launch it onExerciseNameChange = viewModel exerciseNameChange onSaveButtonClick =viewModel validateAndSaveExercise StepDescriptionItems = stepDescriptions onStepDescriptionValueChange = index text gt viewModel modifyStepDescription index text false onStepDescriptionFocusChange = index focusState gt stepDescriptions let if focusState isFocused val stepDescription = it index viewModel modifyStepDescription index stepDescription string false else if it index string isEmpty amp amp it index needFocus == false viewModel removeStepDescriptionAt index onIncreaseDurationButtonClick = viewModel increaseDuration onDecreaseDurationButtonClick = viewModel decreaseDuration onAddNewStepDescriptionClick = viewModel addEmptyStepDescription ``` For my eye this reads clearly what it does It s just passing the parameters that the UI require What could be done that I don t feel necessary is breaking it down into smaller widgets but in the end this UI do require all this parameters I know that there are some obvious improvements here to the way we pass in this parameters but besides this Is there an architecture better than the classic ``` Composable fun TestView action MainActions viewModel OnboardViewModel = getViewModel TestUI onClick = viewModel clickMethod Composable fun TestUI onClick gt Unit ```

Damian Zawadzki · Answer

We don t have that much parameters but i can imagine that you can use one big object state that holds many parts At top lvl you got this one big state and then you pass partial states to each sub component composable This matches our mvi aproach with android view model providing single state on view model Of corse this state could be more complicated and holds many information if to many try to break it to separate view model

Cicero · Answer

Also inside of this UI block everything is broken down like ``` Composable fun FitnessItemDetailUI fitnessDetailItem FitnessItemDetail onClickBack gt Unit = onAddFavorite FitnessItemDetail gt Unit = exoPlayer SimpleExoPlayer isPlaying Boolean onPlayPause gt Unit currentTime Float = 50f totalTime Float = 100f FitnessItemDetailScaffold fitnessDetailItem = fitnessDetailItem onClickBack = onClickBack onAddFavorite = onAddFavorite AppLazyColumn item Column modifier = Modifier fillMaxSize padding gu VideoPlayer exoPlayer = exoPlayer PlayerSliderWithSoundController currentTime totalTime ExerciseTimeController PlayerControllerButtons isPlaying = isPlaying onPlayPauseClick = onPlayPause onFinishTrainingClick = TrainingExercises currentTime = currentTime exercises = fitnessDetailItem exercises Instructions ``` So the insides are not much of a problem

Damian Zawadzki · Answer

as i sadi earlier if there i to many parameters maybe there is a place for improvment by grouping some information with state objects <https github com androidx androidx blob androidx main compose docs compose api guidelines md default policies through hoisted state objects>

Cicero · Answer

Yea but then how this would look we are using this format ``` Composable fun TestView action MainActions viewModel OnboardViewModel = getViewModel TestUI onClick = viewModel clickMethod Composable fun TestUI onClick gt Unit ```

Cicero · Answer

Ok so I could use the same model I described above but break it down in many viewmodels and UI composables

Cicero · Answer

Like ``` Composable fun HeaderView action MainActions viewModel HeaderViewModel = getViewModel HeaderUI onClick = viewModel clickMethod Composable fun HeaderUI onClick gt Unit Composable fun BodyView action MainActions viewModel BodyViewModel = getViewModel BodyUI onClick = viewModel clickMethod Composable fun BodyUI onClick gt Unit ```

Cicero · Answer

I don t mean the body header example explicitly just for visualization purposes

Cicero · Answer

I also understood the idea behind state objects I will keep this in mind as different tools anyway This is not for me There is a question in my company and no one can come up with an answer that everyone agrees Does everyone agree with this

Csaba Szugyiczki · Answer

Definitely not slightly smiling face We in our company have the same opinion as you that we do not want to create huge State objects just to make it easier to pass data down Composables

Damian Zawadzki · Answer

I think that if you are able to separate some logic from components then you should do that But this is my personal opinion and im also curious on other points of view slightly smiling face

Csaba Szugyiczki · Answer

By separating using separate ViewModels sharing state within the same screen will become harder so I am not sure if that would work well with more complicated UIs

Cicero · Answer

But isn t this already separating logic from components ``` Composable fun HeaderView action MainActions viewModel HeaderViewModel = getViewModel HeaderUI onClick = viewModel clickMethod Composable fun HeaderUI onClick gt Unit ```

Cicero · Answer

And yes depending on the structure multiple viewmodels could be a limited option

Cicero · Answer

Sharing view models using multiple views is also an option

Cicero · Answer

I personally don t mind monster UI Composables If their insides are organized

Damian Zawadzki · Answer

well you cannot always skip the complexity

Csaba Kozák · Answer

Huge state object could also negatively effect recompositions see <https kotlinlang slack com archives CJLTWPH7S p1633190913417800 thread ts=1633088247 367200 amp cid=CJLTWPH7S|here>

adjpd · Answer

The docs say you shouldn t include viewmodels as composition locals because a it s hard to test and b the composable isn t so reusable But if you can mock out the viewmodel and your component isn t suppose to be reusable then I don t see why you shoudn t include a viewmodel as a composition local instead of making a composable with 10s of callbacks that are filtered through to other composables

Cicero · Answer

You just shouldn t slightly smiling face

Cicero · Answer

There is zero reason to do it there is no practicality on it that make is an advantage It s harder to test If something makes something else harder to test Even if you are not testing Don t do it Very good rule of thumb Also forget your preview I believe you can t mock a viewmodel inside of your preview Basically don t put your VM into your composable there is no reason for it

raenardev · Answer

As you can see a lot of these are meant to propagate some UI event up to VM ```onMediaDeleteButtonClick gt Unit onOpenGalleryClick gt Unit onTakePhotoClicked gt Unit onCaptureVideoClick gt Unit onSaveButtonClick gt Unit onExerciseNameChange String gt Unit onStepDescriptionValueChange index Int text String gt Unit onStepDescriptionFocusChange index Int focusState FocusState gt Unit onIncreaseDurationButtonClick gt Unit onDecreaseDurationButtonClick gt Unit onAddNewStepDescriptionClick gt Unit``` What if you represent those as Commands Intents Actions Events instead ``` sealed class Action object OpenGallery Action object TakePhoto Action ``` So now you have something that looks like this ``` exerciseName String exerciseNameHasError Boolean durationTimeState State lt String gt StepDescriptionItems List lt StepDescriptionItem gt bitmapState State lt Bitmap gt onAction Action gt Unit ``` If you also wrap other stuff as your UI state then it is even better ``` state State onAction Action gt Unit ```

raenardev · Answer

Usage of state objects is debatable yes I personally think that it is fine to use for most simple cases If it is getting big or if there are many changes to it then you can split out something that changes a lot vs something that relatively stable But as for action intent event handling i think expressing them as sealed hierarchy is the way to go

adjpd · Answer

You can mock a viewmodel in a preview I do And so you can test such composables The advantage and reasons is that you don t spend your time with about 20 callbacks in your composables or by updating ui state object which forces unneeded compositions

Lilly · Answer

< raenardev> Do you see a problem in actions that need parameter Every time you fire such an event you have to instantiate an object e g those events ``` onStepDescriptionValueChange index Int text String gt Unit onStepDescriptionFocusChange index Int focusState FocusState gt Unit sealed class Action object OpenGallery Action object TakePhoto Action data class ChangeStepDescription val index Int val text String Action ```

raenardev · Answer

< Lilly> No i don t see a problem with instantiating objects In this example objects are very lightweight and they are not intended to be instantiated in thousands per second I doubt there will be any measurable overhead from this I would be more worried about complex state data classes which have substates and are modified a lot But not about object instantiating and copying that should not be a problem with modern GC but rather i would be careful with triggering unnecessary recomposition too often But as always measure first and then look for things to optimise slightly smiling face

Lilly · Answer

Thanks for your feedback slightly smiling face