Typeclass example issue

[jeanbaptiste-brasselet]

I was looking at this example from the documentation

const Z = require ('sanctuary-type-classes');

//    Semigroup :: TypeClass
const Semigroup = Z.TypeClass (
  'my-package/Semigroup',
  'http://example.com/my-package#Semigroup',
  [],
  x => x != null && typeof x.concat === 'function'
);

//    concat :: Semigroup a => a -> a -> a
const concat =
def ('concat')
    ({a: [Semigroup]})
    ([a, a, a])
    (x => y => x.concat (y));

concat ([1, 2]) ([3, 4]);
// => [1, 2, 3, 4]

concat ({}) ({});
// ! TypeError: Type-class constraint violation
//
//   concat :: Semigroup a => a -> a -> a
//             ^^^^^^^^^^^    ^
//                            1
//
//   1)  {} :: Object, StrMap ???
//
//   ‘concat’ requires ‘a’ to satisfy the Semigroup type-class constraint; the value at position 1 does not.
//
//   See http://example.com/my-package#Semigroup for information about the my-package/Semigroup type class.

concat (null) (null);
// ! TypeError: Type-class constraint violation
//
//   concat :: Semigroup a => a -> a -> a
//             ^^^^^^^^^^^    ^
//                            1
//
//   1)  null :: Null
//
//   ‘concat’ requires ‘a’ to satisfy the Semigroup type-class constraint; the value at position 1 does not.
//
//   See http://example.com/my-package#Semigroup for information about the my-package/Semigroup type class.

I was wondering why the Semigroup Typeclass was redefined here and why not use directly Z.Semigroup. So I tried and the example didn't work anymore. After some time I found this https://github.com/sanctuary-js/sanctuary-type-classes/blob/master/index.js#L933-L940

And because of this

Z.Semigroup.test({}) //true

Hum I am sure to understand what's going on here, maybe someone with more experience could explain it to me.

[davidchambers]

Thanks for opening this issue, @jeanbaptiste-brasselet.

I was wondering why the Semigroup Typeclass was redefined here and why not use directly Z.Semigroup.

In real code one should certainly use Z.Semigroup rather than define it oneself, but I chose to define Semigroup in the readme to demonstrate the problem type classes solve, in several steps:

1. concat :: String -> String -> String. Only works for strings.
2. concat :: a -> a -> a. Permits nonsensical operations such as concat (null) (null).
3. concat :: Concatenable -> Concatenable -> Concatenable. Permits concat ([]) ('').
4. concat :: Semigroup a => a -> a -> a. No shortcomings. :)

So I tried and the example didn't work anymore.

Good catch. The example is out of date. At some point since it was written we defined concat for objects, so concat ({}) ({}) is now valid. I'll open a pull request to correct the example.