https://kotlinlang.org logo
#announcements
Title
# announcements
g

gildor

09/23/2019, 9:09 AM
I choose approach depending on required name. For your example with state, because you already have suffix “State” everywhere, I would choose first one. But it’s not always necessary
u

ursus

09/23/2019, 9:11 AM
Well I like the nesting declaration of the first one, but callsitea of State.Enabled.Staging.Pending feels weird
But also PendingState would loose to much info, not sure..
g

gildor

09/23/2019, 9:13 AM
State.Enabled.Staging.Pending is definitely huge overkill imo
Why not State.Pending?
again, it depends of course, don’t think that there is unversal rule
u

ursus

09/23/2019, 9:14 AM
Well its a subsubsubstate, you can have more levels of nesting, thats where im not sure on naming
thats was just an one concrete state example out of the tree, ofc its not just nested for sake of nesting
r

Ruckus

09/23/2019, 1:42 PM
If you have that many levels of nesting, maybe you either want to rethink your dependency tree, or sealed classes may not be the right concept. I don't have a huge amount of experience here, but I don't believe I've ever seen an ADT hierarchy that complex. For things like state machines, I often prefer to not use as sealed class for states as I've found it tends to get rather foot-shooty in the long run, but that's just me.
u

ursus

09/23/2019, 9:07 PM
@Ruckus why do you think so? I mean that is proper modeling of the problem I think, for example from my app
Copy code
State {
	Disabled
	Enabled {
		Idle
		MenuShowing
		Staging {
			Idle
			Gallery {
				NoPermission
                Ready {
				    Pending {
						FromPicker
						FromCamera
				    }
                }
			}
			Gifs
		}
	}
}
little bit pseudocode-y but theyre all disjoint so "model states so invalid states are unrepresentable"
r

Ruckus

09/23/2019, 11:45 PM
I was just speaking from my experience, so take it with the appropriate grain of salt. Without knowing more about your domain, I can't say anything about about the "proper" or "optimal" model. There's a number of things you could consider. For example, why do you have the
Enabled
state. It makes
State
into a binary, where one option is singular (
Disabled
) and the other is just a wrapper for other states. What does the
Enabled
state give you that you can't get from
!= Disabled
? (For that matter, you could even consider getting rid of
Disabled
and using a
null
value to represent it, assuming null state isn't already meaningful.)
As for my foot-shooting comment, in my experience, such complex hierarchies are never as final as people expect. So you'll write your code assuming exactly this, as later the inevitable changes come along and all those assumptions go out the window. That's why I prefer to use abstract classes and assume fluidity. Granted, if you know your domain and know that that is 100% absolutely the exact model needed and it's positively guaranteed to never change at all, you can ignore my comments 😄
u

ursus

09/24/2019, 12:07 AM
well those exact implied values I thought were to be removed with sealed classes with them being complete
i.e. null being disabled etc, why not model it? State is Enabled I think is mroe expressive than != Disabled, with other values being on the same semantic level; but yes its semantics
btw I dont see how abstract hierarchies give you more flexibility? You mean like extending to "enum" in some subclass somewhere else? Other than that, if the domain changes you need to make changes anyways
g

gildor

09/24/2019, 1:17 AM
I think all hierarchy of Ready is overkill, why not just Ready state with some data?
u

ursus

09/24/2019, 1:55 AM
yea it has some data to it, it ommited that, sry
g

gildor

09/24/2019, 2:03 AM
Also not sure about
Disabled
state. Why not just move it to the same level as Enabled.Idle and get rid of Enabled and Disabled level, also if you get rid of Ready you will get:
Copy code
Disabled
Idle
MenuShowing
Staging {
    Idle
    Gallery {
      NoPermission
      Ready(some, data)
    }
    Gifs
}
And probly next step would be just get rid of “Staging” And probably gallery (but it depends, do you have or not addfitional nexted cases), and get completely flat states:
Copy code
Disabled
Idle
MenuShowing
StagingIdle
NoPermission
Ready(some, data)
Gifs
Actual implementation of course depends on your case and how you separate your code (probably you want to pass state), but also if you want separate different parts of UI to have own substates, I’m not sure that it should be one huge sealed class, instead some additional sealed class for particular use case
u

ursus

09/24/2019, 2:53 AM
well technically youre not wrong, since you basically picked out all non abstract classes (leaves), but, what is the gain? isnt the nestedness of the hierarchy actually semantically helpful? All states that have the { } in the pseudocode are abstract
with abstract classes havign some sort of common property for its subclasses etc., guess I should have posted the actual code sorry
Copy code
Disabled
Enabled {
	abstract val text

	Idle
	MenuShowing
	Staging {
		abstract val stagedItems

    	JustItems
    	GalleryPicker {
			NoPermission
			Ready(some, data)
    	}
    	GifsPicker {
    		NoInternet
    		Ready(some, data)
    	}
	}
}
would this change something in your mind?
g

gildor

09/24/2019, 3:08 AM
I still don’t understatnd why I need Disabled/Enabled
Staging probably fine, but I would just have one more sealed class for UI that manages this state
u

ursus

09/24/2019, 3:11 AM
Well, its model for the "bottom part" of a chat ui, think like slack, where you can show the menu, gallery etc, and Disabled is when you dont have a business permission to post to given channel
g

gildor

09/24/2019, 3:12 AM
I mean disabled is fine, But I would just inline it and have on the same level as Idle
u

ursus

09/24/2019, 3:12 AM
And I wanted to model it all together because its disjoint, i.e. Disabled should overwrite any current state, so I figured state = Disabled is better then, stagingState = null, whateverState = null, etc
well, all Enabled states have text (current text input)
g

gildor

09/24/2019, 3:13 AM
Noi need to use null, I’m talking about structure of your states and Enabled/Disabled seems just wrong nesting
Even “Idle”?
u

ursus

09/24/2019, 3:14 AM
Yes sorry I replied to your 'staging' mistake message
Idle is when nothing else is showing, just text bar
g

gildor

09/24/2019, 3:14 AM
again, you now arguing by showing some additional info about your case, of course I can say only about what you already shared
Your original question was “how to handle this”, as I said, it up to your case, but what you showing looks incorrect for me as misuse of this approach IMO even if from puristic point of MVI is correct
u

ursus

09/24/2019, 3:16 AM
Im not saying youre wrong, I appreciate the feedback, I just dont see why you want to put Idle and Disabled on same level, as Idle is Enabled which gives it val text
g

gildor

09/24/2019, 3:16 AM
And I would rather have a few null fields than having such bloated states structure
Because of this small thing you make your states strucutre unnecessary complicated imo
so I would just remove text from
enabled
state
and enabled state too
you also can have just some interface implemented by those states that provide text to just avoid this nesting
u

ursus

09/24/2019, 3:18 AM
but then in view you cannot do
when state is Enabled then editText.text = state.text
, but rather case for each branch, duplicating code
g

gildor

09/24/2019, 3:19 AM
and if one day you will need text on disabled state (which perfectly possible for me) all your structure become useless and will be refactored
u

ursus

09/24/2019, 3:19 AM
well, if world changes, model changes, why is that a problem
g

gildor

09/24/2019, 3:20 AM
Copy code
interface StateProvider {
   val text: String
}
editText.text = (state as? StateProvider)?.text
Because you have to change it because of very small feature and you bloated struture of models because of this
u

ursus

09/24/2019, 3:21 AM
how is it bloated, its just inheritance hierarchy
g

gildor

09/24/2019, 3:31 AM
As I said, because Enabled/Disabled as top level state looks unnecessary for me, they do not solve any significant problem
u

ursus

09/24/2019, 3:36 AM
one carries data, other does not, and you solved it via interface, which would be the only super class so..I dont see how is it better, but thank you anyways, I need feedback
btw my original question basically was if, style-wise should one nest substates in parent state (classes), or all top level + postfix
g

gildor

09/24/2019, 3:47 AM
With such structure every style looks bad imo, with prefix or with nesting
3 Views