Skip to content

Instantly share code, notes, and snippets.

@yujikiriki
Last active August 29, 2015 14:04
Show Gist options
  • Save yujikiriki/3d1b98342de4ddc78c44 to your computer and use it in GitHub Desktop.
Save yujikiriki/3d1b98342de4ddc78c44 to your computer and use it in GitHub Desktop.
Hay alguna forma de hacerlo mejor?
case class OrderIncomeReportEntry( month: String, value: Double )
private def sumValues( entries: List[ OrderIncomeReportEntry ] ): List[OrderIncomeReportEntry] = {
val byMonth: Map[ String, List[ OrderIncomeReportEntry ] ] = entries.groupBy( e => e.month )
byMonth.foldLeft( List[OrderIncomeReportEntry]() ) {
( list, value ) =>
val sum = value._2.foldLeft( 0.0 )( ( acc, oire ) => acc + oire.value )
list.+:( OrderIncomeReportEntry( value._1, sum ) )
}
}
Rpta: [{"order":{"month":"08"},"value":200.0},{"order":{"month":"01"},"value":100.0}]
@miguel-vila
Copy link

Estos días estuve pensando muuuucho sobre esto y hubo varias cosas que me rayaron de la solución del monoide. Lo que sigue son algunos pensamientos incompletos:

Lo primero es que pensando la implementación del Monoid me detuve en la definición del elemento identidad y se me hizo feo eso de poner en el mes un string cualquiera ("" en el caso de Juan Pablo). Claro, uno sabe según la forma en la que se define el append puede no importar ese valor en tanto uno lo ignore (por eso es que Juan Pablo puso b.month y no a.month). Además ¿que significa en el dominio el OrderIncomeReportEntry identidad? ¿No debería ser una entidad que tenga sentido instanciar en cualquier contexto y no solamente para hacer calculos?

Entonces por eso es que se me ocurrió que en vez de definir un Monoide uno podría definir un Semigrupo (que exige únicamente implementar la función append). En este caso en particular como el groupBy asegura que está agrupando con listas de por lo menos un elemento (no tiene sentido tener un Map[ String, List[ OrderIncomeReportEntry ] ] con valores de listas vacías) entonces uno no tiene la necesidad de definir un elemento identidad con el que empezar la computación.

Siendo así la cosa podría ser:

implicit object orderSemigroup extends Semigroup[OrderIncomeReportEntry] {
    def append(a: OrderIncomeReportEntry, b: => OrderIncomeReportEntry) = OrderIncomeReportEntry(a.month, a.value + b.value)
}
def sumValuesSemigroup(entries: List[ OrderIncomeReportEntry ] ): List[OrderIncomeReportEntry] = {
    val byMonth: Map[ String, List[ OrderIncomeReportEntry ] ] = entries.groupBy( e => e.month )
    byMonth.mapValues(_.foldMap1Opt().get).values.toList
}

foldMap1Opt , que es un nombre como feo, es la función en scalaz que utiliza un semigrupo sobre un Foldable para sumarlo. Retorna un Some de la suma por que puede que el Foldable sea vacío. En este caso, por la forma en que funciona el groupBy, sabemos que esto nunca va a pasar entonces podemos llamar get libremente. Están esos detalles feos: el nombre de la función y el llamado al get, entonces este acercamiento, si bien es "minimalista" termina con esas torpezas (tal vez haya una función mejor en scalaz, hasta ahora no la he encontrado).

Alejándome del tema un poquito hubiera sido chévere que la librería estandar de scala hubieran definido la función groupBy de forma que los valores no fueran un List sino algo como un NonEmptyList, de forma que en los tipos se hiciese explícito que no va a estar vacia. De ser así en scalaz podría existir un fold que funcionase sobre NonEmptyList y que utilizase un semigrupo. Si así fuera esa función no tendría por que retornar un Option sino de una el resultado.

La otra cosa que me rayó es que en ambas definiciones de append, tanto la del Monoide como la del Semigrupo de arriba, no se está evitando la posibilidad de que se llame con instancias de OrderIncomeReportEntry de meses distintos: ¿Cual debería ser el resultado entonces? ¿Se debería elegir arbitrariamente el mes de alguno de los dos para el resultado?

Digamos que se quiere evitar sumar OrderIncomeReportEntry's de meses distintos. Tal vez este no sea el caso pero la idea es ver que tan lejos se puede llegar. Uno podría hacer que el tipo OrderIncomeReportEntry codificase adicionalmente el mes:

sealed abstract class Month(name: String)
case object January extends Month("January")
case object February extends Month("February")
// etc, ...
case class OrderIncomeReportEntry[M<:Month]( month: M, value: Double )

Entonces ahora los semigrupos están restringidos a operar sobre OrderIncomeReportEntry del mismo tipo y por lo tanto del mismo mes:

implicit def orderSemigroup[M<:Month] = new Semigroup[OrderIncomeReportEntry[M]] {
    def append(a: OrderIncomeReportEntry[M], b: => OrderIncomeReportEntry[M]) = OrderIncomeReportEntry(a.month, a.value + b.value)
}

Entonces el intento de sumar Entries de meses distintos se convierte en un error en tiempo de compilación:

scala> val j1 = OrderIncomeReportEntry(January,2)
j1: OrderIncomeReportEntry[January.type] = OrderIncomeReportEntry(January,2.0)

scala> val j2 = OrderIncomeReportEntry(January,3)
j2: OrderIncomeReportEntry[January.type] = OrderIncomeReportEntry(January,3.0)

scala> j1 |+| j2
res0: OrderIncomeReportEntry[January.type] = OrderIncomeReportEntry(January,5.0)

scala> val f1 = OrderIncomeReportEntry(February,10.0)
f1: OrderIncomeReportEntry[February.type] = OrderIncomeReportEntry(February,10.0)

scala> f1 |+| j1
<console>:22: error: type mismatch;
 found   : OrderIncomeReportEntry[January.type]
 required: OrderIncomeReportEntry[February.type]
              f1 |+| j1
                     ^

Dado esto ¿Como podría definir uno la función sumValues? . Mi sospecha es que el sistema de tipos le ayuda a uno hasta donde empieza el "no determinismo". Sería chévere ver si el compilador se da cuenta que el groupBy en el fondo está segmentando los OrderIncomeReportEntry de acuerdo a su mes, pero lo dudo. No me he puesto a experimentar tanto con eso.

@yujikiriki
Copy link
Author

Señores, me tomó más de 6 meses ponerme al día para poder dar una respuesta al post laaaaaaargo de Vilá. Sin embargo, algunas observaciones de lo que me respondió a la solución propuesta en ese momento:

Lo primero es que pensando la implementación del Monoid me detuve en la >definición del elemento identidad y se me hizo feo eso de poner en el mes un string cualquiera ("" en el caso de Juan Pablo). Claro, uno sabe según la forma en la que se define el append puede no importar ese valor en tanto uno lo ignore (por eso es que Juan Pablo puso b.month y no a.month). Además ¿que significa en el dominio el OrderIncomeReportEntry identidad? ¿No debería ser una entidad que tenga sentido instanciar en cualquier contexto y no solamente para hacer calculos?

Dado el escenario propuesto, pienso que el monoide al tener efecto sólo sobre el campo "value" omite, sin sacrificar la semántica del problema, el campo "month".

Siendo así, creo que en vez de concentrarse en modelar en un ADT los meses como en su ejemplo final

sealed abstract class Month(name: String)
case object January extends Month("January")
case object February extends Month("February")
// etc, ...
case class OrderIncomeReportEntry[M<:Month]( month: M, value: Double )

yo me habría concentrado en modelar el campo "value" como un ADT. Algo así:

case class Money(value: Double)

case class OrderIncomeReportEntry( month: String, value: Money)

y haber creado el monoide sobre Money:

trait Monoid[T] {
  def zero: T
  def op(t1: T, t2: T): T
}

implicit def MoneyAdditionMonoid = new Monoid[Money] {
    def zero = zeroMoney
    def append(m1: Money, m2: Money) = Money(m1.value + m2.value)
}

La cosa que me queda en duda y que no se si se podría hacer es que, dado que partimos de un OrderIncomeReportEntry poder meter dentro de un lense sobre OrderIncomeReportEntry esa suma de Moneys.

Otra opción podría ser crear un tipo genérico "sumable" y crearle su monoide, que la final de cuentas termina siendo nada más que un Monoid[Double] o algo así...

Por otro lado y viendo su propuesta de semigrupos, dado que monoide es "hijo" de semigrupo y que monoide sólo agrega identidad a semigrupo, es tentador dejarlo en términos de semigrupo; solución que quizás sea la que más me convence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment