FlagOp does not satisfy the associative raw
Closed this issue · 2 comments
kazu-yamamoto commented
I have discussed with some friends and now I convinced that FlagOp
should not be an instance of Semigroup
because it does not satisfy the associative raw:
> FlagReset <> (FlagSet <> FlagClear)
FlagKeep
> (FlagReset <> FlagSet) <> FlagClear
FlagClear
I will show alternative APIs soon.
vdukhovni commented
I don't like the proposed change. The only problem was that I failed to pay close attention to an incorrect definition of the (<>) operation for "FlagReset". This was not a problem with the original Maybe (Maybe Boolean)
and the correct definition was lost in translation to the more usable flags. The fix is:
data FlagOp = FlagKeep
| FlagClear
| FlagSet
| FlagReset
instance Semigroup FlagOp where
FlagKeep <> op = op
op <> _ = op
applyOp :: FlagOp -> Bool -> Bool
applyOp FlagSet _ = True
applyOp FlagClear _ = False
applyOp _ v = v
with that we regain associativity:
FlagKeep <> (a <> b) = a <> b
(FlagKeep <> a) <> b = a <> b
FlagSet <> (a <> b) = FlagSet
(FlagSet <> a) <> b = FlagSet <> b = FlagSet
FlagClear <> (a <> b) = FlagClear
(FlagClear <> a) <> b = FlagClear <> b = FlagSet
FlagReset <> (a <> b) = FlagReset
(FlagReset <> a) <> b = FlagReset <> b = FlagReset
And just need to delay the interpretation of FlagReset
as a NOOP until we finally apply it to a boolean, but not change it to FlagKeep
while composing the operations.
kazu-yamamoto commented
Fixed via #121.