ekmett/keys

Instances for Sum and :+: are inconsistent

Closed this issue · 2 comments

type instance Key (Functor.Sum f g) = (Key f, Key g)
type instance Key (f :+: g)         = Either (Key f) (Key g)

Is this intentional, given that Sum and :+: are essentially the same type?

Oh, I didn't realize that the :+: instances were written by someone else.

To be honest, I feel like Either (Key f) (Key g) is a better choice than (Key f, Key g). AFAICT, using (Key f, Key g) makes it impossible to give Keyed, FoldableWithKey(1), and TraversableWithKey(1) instances for Sum, and it would be more consistent with the treatment for other sum types in keys. For instance, Key [] = Int, which allows you to choose which variant of the sum you wish to index into.

Plus, the Adjustable instance for :+::

instance (Adjustable f, Adjustable g) => Adjustable (f :+: g) where
  adjust h (Left a) (L1 fa) = L1 (adjust h a fa)
  adjust h (Right b) (R1 fb) = R1 (adjust h b fb)
  adjust _ _ x = x
  replace (Left a) v (L1 fa) = L1 (replace a v fa)
  replace (Right b) v (R1 fb) = R1 (replace b v fb)
  replace _ _ x = x

is much spiritually closer to the Adjustable [] instance

instance Adjustable [] where
  adjust f 0 (x:xs) = f x : xs
  adjust _ _ [] = []
  adjust f n (x:xs) = n' `seq` x : adjust f n' xs where n' = n - 1

than the Adjustable instance for Sum is:

instance (Adjustable f, Adjustable g) => Adjustable (Functor.Sum f g) where
  adjust f (x,_) (Functor.InL a) = Functor.InL (adjust f x a)
  adjust f (_,y) (Functor.InR b) = Functor.InR (adjust f y b)

The downside to switching over to Either (Key f) (Key g) for Sum would be that its Lookup and Indexable instances would need to change. I could imagine something like this:

instance (Lookup f, Lookup g) => Lookup (Sum f g) where
  lookup (Left x)  (InL a) = Data.Key.lookup x a
  lookup (Right y) (InR b) = Data.Key.lookup y b
  lookup _          _      = Nothing

Similarly for Indexable.