4
\$\begingroup\$

I attempted to define Penultimate, i.e. a type class for shapeless.HList's that have a second to last element.

import shapeless.{HList, HNil, ::}

object Penultimate {
  type Aux[L <: HList, O] = Penultimate[L] { type Out = O }

  def apply[H <: HList](implicit ev: Penultimate[H]) = ev

  implicit def secondToLast[H, G]: Aux[H :: G :: HNil, H] =
    new Penultimate[H :: G :: HNil] {
      override type Out = H
      override def apply(in: H :: G :: HNil): Out = in.head
    }

  implicit def inductive[H, T <: HList, OutT](
     implicit penult: Aux[T, OutT]
  ): Penultimate[H :: T] = new Penultimate[::[H, T]] {

    override type Out = OutT

    override def apply(in: H :: T): Out = penult.apply(in.tail)
  }
}

trait Penultimate[H <: HList] {
  type Out
  def apply(in: H): Out
}

It appears to work:

scala> Penultimate[Int :: Int :: HNil]
res1: net.Penultimate[shapeless.::[Int,shapeless.::[Int,shapeless.HNil]]] = net.Penultimate$$anon$1@3c37902d

scala> Penultimate[Int :: HNil]
<console>:16: error: could not find implicit value for parameter ev: net.Penultimate[shapeless.::[Int,shapeless.HNil]]
       Penultimate[Int :: HNil]
                  ^

scala> Penultimate[HNil]
<console>:16: error: could not find implicit value for parameter ev: net.Penultimate[shapeless.HNil]
       Penultimate[HNil]
                  ^

But, is it a valid implementation of Penultimate? If not, what flaws does it have?

\$\endgroup\$
3
  • 1
    \$\begingroup\$ @Jamal Why remove the shapeless tag? I'm not as familiar with Code Review's conventions as much as SO proper's, but this is a very Shapeless-specific question and the tag seems useful for discovery. \$\endgroup\$ Commented Jan 19, 2017 at 1:13
  • \$\begingroup\$ @TravisBrown: We usually don't need very many specific tags, plus there aren't any other questions relevant to it now. \$\endgroup\$
    – Jamal
    Commented Jan 19, 2017 at 1:48
  • \$\begingroup\$ "... plus there aren't any other questions relevant to it now." What makes you say that? Also, what's the harm in adding a tag that's pertinent to the Code Review question? \$\endgroup\$ Commented Jan 19, 2017 at 2:37

1 Answer 1

7
\$\begingroup\$

More static types

One big improvement you could make would be to have the Penultimate.apply method provide the output type in its signature. For example, with your current code you can write this:

scala> val instance = Penultimate[Int :: String :: HNil]
instance: Penultimate[shapeless.::[Int,shapeless.::[String,shapeless.HNil]]] = Penultimate$$anon$1@2819ef2f

scala> instance(1 :: "" :: HNil)
res0: instance.Out = 1

But not this:

scala> val x: Int = instance(1 :: "" :: HNil)
<console>:14: error: type mismatch;
 found   : instance.Out
 required: Int
       val x: Int = instance(1 :: "" :: HNil)
                            ^

If you want to keep track of the output type statically, you have to do it by hand:

val instance = implicitly[Penultimate.Aux[Int :: String :: HNil, Int]]

val x: Int = instance(1 :: "" :: HNil)

Which works, but ugh. It's a lot easier to change your implementation of apply:

def apply[H <: HList](implicit ev: Penultimate[H]): Aux[H, ev.Out] = ev

Now this works just fine:

scala> Penultimate[Int :: String :: HNil].apply(1 :: "" :: HNil): Int
res1: Int = 1

…well, at least it works for hlists with two elements. When we add more things get weird:

scala> val instance = Penultimate[Char :: Int :: String :: HNil]
warning: there was one feature warning; re-run with -feature for details
instance: Penultimate[shapeless.::[Char,shapeless.::[Int,shapeless.::[String,shapeless.HNil]]]]{type Out = ev.Out} forSome { val ev: Penultimate[shapeless.::[Char,shapeless.::[Int,shapeless.::[String,shapeless.HNil]]]] } = Penultimate$$anon$2@49393eeb

scala> val x: Int = instance('a' :: 1 :: "" :: HNil)
<console>:18: error: type mismatch;
 found   : instance.Out
    (which expands to)  ev.Out
 required: Int
       val x: Int = instance('a' :: 1 :: "" :: HNil)
                            ^

This time apply is fine, but the method that provides inductive instances isn't. You can fix it by giving its result type the same kind of treatment:

implicit def inductive[H, T <: HList, OutT](
   implicit penult: Aux[T, OutT]
): Aux[H :: T, OutT] = new Penultimate[H ::  T] {
  override type Out = OutT

  override def apply(in: H :: T): Out = penult.apply(in.tail)
}

Now the following will work just fine:

val instance = Penultimate[Char :: Int :: String :: HNil]
val x: Int = instance('a' :: 1 :: "" :: HNil)

Now we have useful result types for instances (i.e. result types that track the output type statically), and an improved apply that lets us make use of them.

Point of style 1: DepFn1

This is a relatively minor point, but you could define your type class like this:

import shapeless.DepFn1

trait Penultimate[H <: HList] extends DepFn1[H]

This is essentially exactly the same (the Out and apply are still there—you're just getting them from DepFn1), but it's less verbose and clearer about the intent.

Point of style 2: infix vs. not infix

I'd personally avoid writing ::[H, T] in one place and H :: T in others—for a type with a symbolic name like :: I'd always use the infix version. If you prefer the non-infix version, that's also fine, I guess. Mixing them isn't fine, though. :)

Point of style 3: don't just stick the override keyword everywhere

This is again a matter of personal preference, but I'd drop the override keyword from both the type member and the apply implementations in the anonymous new Penultimate class definitions, since your implementations aren't actually overriding anything.

Point of style 4: consistent type parameter names

In several places you use H as a type parameter name for a head type parameter, but in the Penultimate trait definition itself you use it apparently to stand just for "hlist". I'd use L, which also has the benefit of being consistent with the L in the Aux type member.

Point of style 5: explicit apply

Now we're getting into super-nitpick territory, but in general if writing out x.apply(y) isn't necessary to avoid y looking like an explicit implicit parameter, etc., I'd just use x(y).

Point of style 6: import order

More nitpickery: : precedes H, so it's likely that if you're using something like Scalastyle it's going to complain about the import.

Point of style 7: trait vs. abstract class

For a type class like this that will almost certainly never need to be mixed in with another trait, I'd prefer to make it an abstract class, for the reasons given here.

Point of style 8: unique instance names

I'm not sure how strongly I'd be willing to stand by this one if pressed, but I've developed a distaste for type class instance names that I can't reasonably expect to be globally unique. It's very unlikely anyone will be importing the contents of the Penultimate type class here, but if they did, having something called inductive in scope is kind of unhelpful. At the very least I'd probably tack a Penultimate suffix on each method name. Maybe that's unreasonable. I don't know—it probably doesn't matter much either way.

Other than that it looks pretty good to me!

\$\endgroup\$
1
  • 1
    \$\begingroup\$ It'd be great to see some of these comments make it to the shapeless Guide as there are some great subtleties being explained here that I've been curious about for a while. \$\endgroup\$
    – tksfz
    Commented Jan 19, 2017 at 0:52

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.