Open Aimarekin opened 2 years ago
This construct is often referred to as a switch statement or a match statement, so I'll refer to this object as Match
for brevity, and I'll also rename this suggestion accordingly so it can be searched more easily.
We've actually had a similar suggestion for a 'Mapping' object before. The main concern I had with that suggestion was computational cost - it's very easy to accidentally require computing all results, even though only one will be used. That being said, this feature request seems to get around that issue, so it looks alright.
Of course, using constants shouldn't be a problem - nothing needs to be computed dynamically:
local visible = Value(false)
local blurSize = Match(visible) {
[true] = 24,
[false] = 0
}
We need to be careful about how we approach dynamic values though. If we just pass in Computed objects like this, then we end up having to recalculate all the state objects (that is, unless we opt to switch Fusion to a lazy 'pull' model as suggested in #144):
local visible = Value(False)
local aperture = Value(0.5)
local blurSize = Match(visible) {
-- even if visible is false, changing `aperture` will cause this function to rerun!
[true] = Computed(function(use)
return calculateBlurExpensively(use(aperture))
end),
[false] = 0,
}
In lieu of lazy evaluation, we could instead make Match
behave like a multi-arm Computed
object by passing in callbacks directly:
local visible = Value(False)
local aperture = Value(0.5)
local blurSize = Match(visible) {
-- even if visible is false, changing `aperture` will cause this function to rerun!
[true] = function(use)
return calculateBlurExpensively(use(aperture))
end,
[false] = 0,
}
If none of these options work, then we could rule out dynamic values out explicitly, though this would be somewhat inconvenient.
There's also the case of handling nil
, which as you point out requires a None
symbol to handle correctly. We could possibly add a separate Fallback
state object to handle matching and replacing nils, though admittedly it would probably be more elegant to represent all matching behaviour under Match
:
local message = Value("Hello, world")
local withDefault = Fallback(message, "(no messages to show)")
I don't like the idea of switching the object into a different 'mode' for doing inequality comparisons. It would either be better to spin off into something like a MatchRange
object:
local grade = Value(73.2)
local report = MatchRange(grade) {
[{0, 70}] = "Fail",
[{70, 100}] = "Pass"
}
Or alternatively, we could generalise Match
to accept special keys for custom predicates, a bit like how New
and Hydrate
accept special keys for instance operations:
local grade = Value(73.2)
local report = Match(grade) {
[LessThan(70)] = "Fail",
[GreaterOrEqual(70)] = "Pass"
}
Or with range predicates:
local grade = Value(73.2)
local report = Match(grade) {
[InRange(0, 70)] = "Fail",
[InRange(70, 100)] = "Pass"
}
Or alternatively with a Default
case:
local grade = Value(73.2)
local report = Match(grade) {
[Default] = "Fail",
[GreaterOrEqual(70)] = "Pass"
}
I think we could do some quite rich pattern matching with this approach, and this also opens the door to user-defined predicates in case more advanced behaviour (such as regex matching) is desired.
I'm not entirely sure how I feel about it. It could definitely be useful, but I'm pretty sure 99% of the matching I've done personally is on booleans lol.
This construct is often referred to as a switch statement or a match statement, so I'll refer to this object as
Match
for brevity, and I'll also rename this suggestion accordingly so it can be searched more easily.
Didn't think of the similarity with switch statements! Thanks
We need to be careful about how we approach dynamic values though. If we just pass in Computed objects like this, then we end up having to recalculate all the state objects (that is, unless we opt to switch Fusion to a lazy 'pull' model as suggested in #144):
local visible = Value(False) local aperture = Value(0.5) local blurSize = Match(visible) { -- even if visible is false, changing `aperture` will cause this function to rerun! [true] = Computed(function(use) return calculateBlurExpensively(use(aperture)) end), [false] = 0, }
EDIT: Realized this is not what the issue was about. Made another comment about it. I think this will not be necessary. The
Match
object will only need to recalculate a value if it is currently being used. So, ifvisisble
isfalse
, it will only need to mark the value it's currently returning as a dependency. Because0
is a constant, nothing will be marked as a dependency (exceptvisible
, of course). However, ifvisible
changes totrue
, then theComputed
will be marked as a dependency. When theMatch
gets a request to update, it can check if the previous key to match is the same, and if it is, get the value of theComputed
and return it. If the key has changed, it will get the value it has to return from the table, and update the dependencies. In lieu of lazy evaluation, we could instead makeMatch
behave like a multi-armComputed
object by passing in callbacks directly:local visible = Value(False) local aperture = Value(0.5) local blurSize = Match(visible) { -- even if visible is false, changing `aperture` will cause this function to rerun! [true] = function(use) return calculateBlurExpensively(use(aperture)) end, [false] = 0, }
I think the former is an acceptable approach
There's also the case of handling
nil
, which as you point out requires aNone
symbol to handle correctly. We could possibly add a separateFallback
state object to handle matching and replacing nils, though admittedly it would probably be more elegant to represent all matching behaviour underMatch
:local message = Value("Hello, world") local withDefault = Fallback(message, "(no messages to show)")
I think I've heard of this
IfNil
object before, and ofIfError
. I thinkIfNil
/Fallback
could be their own objects to make them more explicit, easier to use and optimal. I don't like the idea of switching the object into a different 'mode' for doing inequality comparisons. It would either be better to spin off into something like aMatchRange
object:...
Or with range predicates:
local grade = Value(73.2) local report = Match(grade) { [InRange(0, 70)] = "Fail", [InRange(70, 100)] = "Pass" }
This is the option I like most. Using a simple table to define ranges could cause confusion if Fusion thinks some random table is similar to a range. Plus this one seems more optimal. Here's how I would make it work:
local grade = Value(73.2)
local report = Match(grade) {
[0] = "Bummer!"
[InRange(0, 70)] = "Fail",
[50] = "In the middle!",
[InRange(70, 100)] = "Pass"
[MoreThan(100)] = "Wonderful!"
}
Fusion will separate this table into three, first one with the explicit keys (0
and 50
), the second one an array with each range in order, and the third one a dictionary with each range and it's assigned value. This way Fusion will first go through any explicit keys, giving them top priority, and then loop through the array, find the first range it fits into and return it's value in the third table.
It might seem expensive to calculate, but some optimizations can be done. As I said earlier, calculating which key matches will only have to be done when the matching state changes. Then we could do some optimizations, like first checking if the range that was previously matching still matches.
Another issue is how we will handle inclusive ranges. Does InRange(0,70)
also include 0 and 70? Or just one of them? Which one and why? If it doesn't match any of them and I would like them to, I'll have to explicitly assign both 0 and 70. Isn't that repetitive? This could cause some frustrating errors.
About using states as keys: it might also seem expensive to do, but mind that it will only be necessary to recalculate keys if either the matched state changes, or the key currently matched changes. Then, keys will have to be recalculated one by one until one matches (not all of them). This can still get very expensive to do, and devs should be warned (or maybe the feature should not be included to avoid this issue).
I think this will not be necessary. The
Match
object will only need to recalculate a value if it is currently being used. So, ifvisisble
isfalse
, it will only need to mark the value it's currently returning as a dependency. Because0
is a constant, nothing will be marked as a dependency (exceptvisible
, of course). However, ifvisible
changes totrue
, then theComputed
will be marked as a dependency. When theMatch
gets a request to update, it can check if the previous key to match is the same, and if it is, get the value of theComputed
and return it. If the key has changed, it will get the value it has to return from the table, and update the dependencies.
Realized that the real issue is that the Computed
s that are not being actively used will update themselves, despite not being really linked to anything at that moment. Indeed, I think a switch to lazy state pulling as #144 suggests will be very beneficial to Fusion.
I think we could go ahead with this if we were to switch Fusion to a lazy evaluation model :)
Contingent on the outcome of #144
Fusion will be switching to lazy evaluation. Once that's in, I'll unblock this.
I'm generally still in favour of implementing this with predicates.
I'd say function keys should be interpreted as predicates of the form (Use, T) -> boolean
. That should make predicates easy to define and insert.
Other than that, we should interpret any other keys as values to match against. If given a state object, should probably unwrap. Probably best to test against these first, though predicate ordering is still an interesting question.
We could implement the nil check as a "nil predicate".
Something else to consider: we should be wary of namespace pollution here. Probably best to come up with a predicate naming scheme of some kind. MatchNil
, MatchRange
, MatchAbove
, etc...
Currently, if you'd like to set a property to a value depending on the value of another state, you have to use a ComputedObject:
Because this occurs often with many of my GUIs, I think a way to do it natively would fit in more with Fusion's "ideology". This could be done with a "correspondence" state object that updates every time another state changes, and returns a value according to that state. For example:
This correspondence object could be further modified to add an else:
Or get the value of the highest key that is lower than the state:
Or make the values other states, so you can compute the value returned. Even the keys themselves could be states. The
None
symbol will be useful here to return nil for specific keys.I have already made this for use on my own projects - if it is approved, I can make a PR (which will probably have to be reviewed to make sure I didn't mess up typecheckings)