kazu-yamamoto/dns

FlagOp does not satisfy the associative raw

Closed this issue · 2 comments

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.

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.

Fixed via #121.