Option folding idiom

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Option folding idiom

Mark Hamstra
In code added to Spark over the past several months, I'm glad to see more
use of `foreach`, `for`, `map` and `flatMap` over `Option` instead of
pattern matching boilerplate.  There are opportunities to push `Option`
idioms even further now that we are using Scala 2.10 in master, but I want
to discuss the issue here a little bit before committing code whose form
may be a little unfamiliar to some Spark developers.

In particular, I really like the use of `fold` with `Option` to cleanly an
concisely express the "do something if the Option is None; do something
else with the thing contained in the Option if it is Some" code fragment.

An example:

Instead of...

val driver = drivers.find(_.id == driverId)
driver match {
  case Some(d) =>
    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
    else {
      d.worker.foreach { w =>
        w.actor ! KillDriver(driverId)
      }
    }
    val msg = s"Kill request for $driverId submitted"
    logInfo(msg)
    sender ! KillDriverResponse(true, msg)
  case None =>
    val msg = s"Could not find running driver $driverId"
    logWarning(msg)
    sender ! KillDriverResponse(false, msg)
}

...using fold we end up with...

driver.fold
  {
    val msg = s"Could not find running driver $driverId"
    logWarning(msg)
    sender ! KillDriverResponse(false, msg)
  }
  { d =>
    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
    else {
      d.worker.foreach { w =>
        w.actor ! KillDriver(driverId)
      }
    }
    val msg = s"Kill request for $driverId submitted"
    logInfo(msg)
    sender ! KillDriverResponse(true, msg)
  }


So the basic pattern (and my proposed formatting standard) for folding over
an `Option[A]` from which you need to produce a B (which may be Unit if
you're only interested in side effects) is:

anOption.fold
  {
    // something that evaluates to a B if anOption = None
  }
  { a =>
    // something that transforms `a` into a B if anOption = Some(a)
  }


Any thoughts?  Does anyone really, really hate this style of coding and
oppose its use in Spark?
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Michael (Bach) Bui
+1.

It is a little bit harder to read for new comers but not really a big deal.




On Dec 26, 2013, at 2:33 PM, Mark Hamstra <[hidden email]> wrote:

> In code added to Spark over the past several months, I'm glad to see more
> use of `foreach`, `for`, `map` and `flatMap` over `Option` instead of
> pattern matching boilerplate.  There are opportunities to push `Option`
> idioms even further now that we are using Scala 2.10 in master, but I want
> to discuss the issue here a little bit before committing code whose form
> may be a little unfamiliar to some Spark developers.
>
> In particular, I really like the use of `fold` with `Option` to cleanly an
> concisely express the "do something if the Option is None; do something
> else with the thing contained in the Option if it is Some" code fragment.
>
> An example:
>
> Instead of...
>
> val driver = drivers.find(_.id == driverId)
> driver match {
>  case Some(d) =>
>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
>    else {
>      d.worker.foreach { w =>
>        w.actor ! KillDriver(driverId)
>      }
>    }
>    val msg = s"Kill request for $driverId submitted"
>    logInfo(msg)
>    sender ! KillDriverResponse(true, msg)
>  case None =>
>    val msg = s"Could not find running driver $driverId"
>    logWarning(msg)
>    sender ! KillDriverResponse(false, msg)
> }
>
> ...using fold we end up with...
>
> driver.fold
>  {
>    val msg = s"Could not find running driver $driverId"
>    logWarning(msg)
>    sender ! KillDriverResponse(false, msg)
>  }
>  { d =>
>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
>    else {
>      d.worker.foreach { w =>
>        w.actor ! KillDriver(driverId)
>      }
>    }
>    val msg = s"Kill request for $driverId submitted"
>    logInfo(msg)
>    sender ! KillDriverResponse(true, msg)
>  }
>
>
> So the basic pattern (and my proposed formatting standard) for folding over
> an `Option[A]` from which you need to produce a B (which may be Unit if
> you're only interested in side effects) is:
>
> anOption.fold
>  {
>    // something that evaluates to a B if anOption = None
>  }
>  { a =>
>    // something that transforms `a` into a B if anOption = Some(a)
>  }
>
>
> Any thoughts?  Does anyone really, really hate this style of coding and
> oppose its use in Spark?

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Christopher Nguyen
+1 as you can't fight the future, but clear warning signs ahead would be
helpful :)

Just be careful that it's not an exact equivalent to *match*, else we can
get confused by behavior like this:


*scala> class parentdefined class parent*


*scala> class child1 extends parentdefined class child1*


*scala> class child2 extends parentdefined class child2*


*scala> Option("abc") match { case None => new child1 ; case _ => new
child2 } res16: parent = child2@9d9347d*





*scala> Option("abc").fold { new child1 } { _ => new child2 }<console>:11:
error: type mismatch;  found   : child2 required: child1
 Option("abc").fold { new child1 } { _ => new child2 }*


*scala> Option("abc").fold { new child1 : parent } { _ => new child2 }
res14: parent = child2@58a470a8*

--
Christopher T. Nguyen
Co-founder & CEO, Adatao <http://adatao.com>
linkedin.com/in/ctnguyen



On Thu, Dec 26, 2013 at 12:54 PM, Michael (Bach) Bui <[hidden email]>wrote:

> +1.
>
> It is a little bit harder to read for new comers but not really a big deal.
>
>
>
>
> On Dec 26, 2013, at 2:33 PM, Mark Hamstra <[hidden email]> wrote:
>
> > In code added to Spark over the past several months, I'm glad to see more
> > use of `foreach`, `for`, `map` and `flatMap` over `Option` instead of
> > pattern matching boilerplate.  There are opportunities to push `Option`
> > idioms even further now that we are using Scala 2.10 in master, but I
> want
> > to discuss the issue here a little bit before committing code whose form
> > may be a little unfamiliar to some Spark developers.
> >
> > In particular, I really like the use of `fold` with `Option` to cleanly
> an
> > concisely express the "do something if the Option is None; do something
> > else with the thing contained in the Option if it is Some" code fragment.
> >
> > An example:
> >
> > Instead of...
> >
> > val driver = drivers.find(_.id == driverId)
> > driver match {
> >  case Some(d) =>
> >    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> >    else {
> >      d.worker.foreach { w =>
> >        w.actor ! KillDriver(driverId)
> >      }
> >    }
> >    val msg = s"Kill request for $driverId submitted"
> >    logInfo(msg)
> >    sender ! KillDriverResponse(true, msg)
> >  case None =>
> >    val msg = s"Could not find running driver $driverId"
> >    logWarning(msg)
> >    sender ! KillDriverResponse(false, msg)
> > }
> >
> > ...using fold we end up with...
> >
> > driver.fold
> >  {
> >    val msg = s"Could not find running driver $driverId"
> >    logWarning(msg)
> >    sender ! KillDriverResponse(false, msg)
> >  }
> >  { d =>
> >    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> >    else {
> >      d.worker.foreach { w =>
> >        w.actor ! KillDriver(driverId)
> >      }
> >    }
> >    val msg = s"Kill request for $driverId submitted"
> >    logInfo(msg)
> >    sender ! KillDriverResponse(true, msg)
> >  }
> >
> >
> > So the basic pattern (and my proposed formatting standard) for folding
> over
> > an `Option[A]` from which you need to produce a B (which may be Unit if
> > you're only interested in side effects) is:
> >
> > anOption.fold
> >  {
> >    // something that evaluates to a B if anOption = None
> >  }
> >  { a =>
> >    // something that transforms `a` into a B if anOption = Some(a)
> >  }
> >
> >
> > Any thoughts?  Does anyone really, really hate this style of coding and
> > oppose its use in Spark?
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Evan Chan
In reply to this post by Mark Hamstra
+1 for using more functional idioms in general.

That's a pretty clever use of `fold`, but putting the default condition
first there makes it not as intuitive.   What about the following, which
are more readable?

    option.map { a => someFuncMakesB() }
              .getOrElse(b)

    option.map { a => someFuncMakesB() }
              .orElse { a => otherDefaultB() }.get


On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <[hidden email]>wrote:

> In code added to Spark over the past several months, I'm glad to see more
> use of `foreach`, `for`, `map` and `flatMap` over `Option` instead of
> pattern matching boilerplate.  There are opportunities to push `Option`
> idioms even further now that we are using Scala 2.10 in master, but I want
> to discuss the issue here a little bit before committing code whose form
> may be a little unfamiliar to some Spark developers.
>
> In particular, I really like the use of `fold` with `Option` to cleanly an
> concisely express the "do something if the Option is None; do something
> else with the thing contained in the Option if it is Some" code fragment.
>
> An example:
>
> Instead of...
>
> val driver = drivers.find(_.id == driverId)
> driver match {
>   case Some(d) =>
>     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
>     else {
>       d.worker.foreach { w =>
>         w.actor ! KillDriver(driverId)
>       }
>     }
>     val msg = s"Kill request for $driverId submitted"
>     logInfo(msg)
>     sender ! KillDriverResponse(true, msg)
>   case None =>
>     val msg = s"Could not find running driver $driverId"
>     logWarning(msg)
>     sender ! KillDriverResponse(false, msg)
> }
>
> ...using fold we end up with...
>
> driver.fold
>   {
>     val msg = s"Could not find running driver $driverId"
>     logWarning(msg)
>     sender ! KillDriverResponse(false, msg)
>   }
>   { d =>
>     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
>     else {
>       d.worker.foreach { w =>
>         w.actor ! KillDriver(driverId)
>       }
>     }
>     val msg = s"Kill request for $driverId submitted"
>     logInfo(msg)
>     sender ! KillDriverResponse(true, msg)
>   }
>
>
> So the basic pattern (and my proposed formatting standard) for folding over
> an `Option[A]` from which you need to produce a B (which may be Unit if
> you're only interested in side effects) is:
>
> anOption.fold
>   {
>     // something that evaluates to a B if anOption = None
>   }
>   { a =>
>     // something that transforms `a` into a B if anOption = Some(a)
>   }
>
>
> Any thoughts?  Does anyone really, really hate this style of coding and
> oppose its use in Spark?
>



--
--
Evan Chan
Staff Engineer
[hidden email]  |

<http://www.ooyala.com/>
<http://www.facebook.com/ooyala><http://www.linkedin.com/company/ooyala><http://www.twitter.com/ooyala>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Mark Hamstra
On the contrary, it is the completely natural place for the initial value
of the accumulator, and provides the expected result of folding over an
empty collection.

scala> val l: List[Int] = List()

l: List[Int] = List()


scala> l.fold(42)(_ + _)

res0: Int = 42


scala> val o: Option[Int] = None

o: Option[Int] = None


scala> o.fold(42)(_ + 1)

res1: Int = 42


On Thu, Dec 26, 2013 at 5:51 PM, Evan Chan <[hidden email]> wrote:

> +1 for using more functional idioms in general.
>
> That's a pretty clever use of `fold`, but putting the default condition
> first there makes it not as intuitive.   What about the following, which
> are more readable?
>
>     option.map { a => someFuncMakesB() }
>               .getOrElse(b)
>
>     option.map { a => someFuncMakesB() }
>               .orElse { a => otherDefaultB() }.get
>
>
> On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <[hidden email]
> >wrote:
>
> > In code added to Spark over the past several months, I'm glad to see more
> > use of `foreach`, `for`, `map` and `flatMap` over `Option` instead of
> > pattern matching boilerplate.  There are opportunities to push `Option`
> > idioms even further now that we are using Scala 2.10 in master, but I
> want
> > to discuss the issue here a little bit before committing code whose form
> > may be a little unfamiliar to some Spark developers.
> >
> > In particular, I really like the use of `fold` with `Option` to cleanly
> an
> > concisely express the "do something if the Option is None; do something
> > else with the thing contained in the Option if it is Some" code fragment.
> >
> > An example:
> >
> > Instead of...
> >
> > val driver = drivers.find(_.id == driverId)
> > driver match {
> >   case Some(d) =>
> >     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> >     else {
> >       d.worker.foreach { w =>
> >         w.actor ! KillDriver(driverId)
> >       }
> >     }
> >     val msg = s"Kill request for $driverId submitted"
> >     logInfo(msg)
> >     sender ! KillDriverResponse(true, msg)
> >   case None =>
> >     val msg = s"Could not find running driver $driverId"
> >     logWarning(msg)
> >     sender ! KillDriverResponse(false, msg)
> > }
> >
> > ...using fold we end up with...
> >
> > driver.fold
> >   {
> >     val msg = s"Could not find running driver $driverId"
> >     logWarning(msg)
> >     sender ! KillDriverResponse(false, msg)
> >   }
> >   { d =>
> >     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> >     else {
> >       d.worker.foreach { w =>
> >         w.actor ! KillDriver(driverId)
> >       }
> >     }
> >     val msg = s"Kill request for $driverId submitted"
> >     logInfo(msg)
> >     sender ! KillDriverResponse(true, msg)
> >   }
> >
> >
> > So the basic pattern (and my proposed formatting standard) for folding
> over
> > an `Option[A]` from which you need to produce a B (which may be Unit if
> > you're only interested in side effects) is:
> >
> > anOption.fold
> >   {
> >     // something that evaluates to a B if anOption = None
> >   }
> >   { a =>
> >     // something that transforms `a` into a B if anOption = Some(a)
> >   }
> >
> >
> > Any thoughts?  Does anyone really, really hate this style of coding and
> > oppose its use in Spark?
> >
>
>
>
> --
> --
> Evan Chan
> Staff Engineer
> [hidden email]  |
>
> <http://www.ooyala.com/>
> <http://www.facebook.com/ooyala><http://www.linkedin.com/company/ooyala><
> http://www.twitter.com/ooyala>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

rxin
I'm not strongly against Option.fold, but I find the readability getting
worse for the use case you brought up.  For the use case of if/else, I find
Option.fold pretty confusing because it reverses the order of Some vs None.
Also, when code gets long, the lack of an obvious boundary (the only
boundary is "} {") with two closures is pretty confusing.


On Thu, Dec 26, 2013 at 4:23 PM, Mark Hamstra <[hidden email]>wrote:

> On the contrary, it is the completely natural place for the initial value
> of the accumulator, and provides the expected result of folding over an
> empty collection.
>
> scala> val l: List[Int] = List()
>
> l: List[Int] = List()
>
>
> scala> l.fold(42)(_ + _)
>
> res0: Int = 42
>
>
> scala> val o: Option[Int] = None
>
> o: Option[Int] = None
>
>
> scala> o.fold(42)(_ + 1)
>
> res1: Int = 42
>
>
> On Thu, Dec 26, 2013 at 5:51 PM, Evan Chan <[hidden email]> wrote:
>
> > +1 for using more functional idioms in general.
> >
> > That's a pretty clever use of `fold`, but putting the default condition
> > first there makes it not as intuitive.   What about the following, which
> > are more readable?
> >
> >     option.map { a => someFuncMakesB() }
> >               .getOrElse(b)
> >
> >     option.map { a => someFuncMakesB() }
> >               .orElse { a => otherDefaultB() }.get
> >
> >
> > On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <[hidden email]
> > >wrote:
> >
> > > In code added to Spark over the past several months, I'm glad to see
> more
> > > use of `foreach`, `for`, `map` and `flatMap` over `Option` instead of
> > > pattern matching boilerplate.  There are opportunities to push `Option`
> > > idioms even further now that we are using Scala 2.10 in master, but I
> > want
> > > to discuss the issue here a little bit before committing code whose
> form
> > > may be a little unfamiliar to some Spark developers.
> > >
> > > In particular, I really like the use of `fold` with `Option` to cleanly
> > an
> > > concisely express the "do something if the Option is None; do something
> > > else with the thing contained in the Option if it is Some" code
> fragment.
> > >
> > > An example:
> > >
> > > Instead of...
> > >
> > > val driver = drivers.find(_.id == driverId)
> > > driver match {
> > >   case Some(d) =>
> > >     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> > >     else {
> > >       d.worker.foreach { w =>
> > >         w.actor ! KillDriver(driverId)
> > >       }
> > >     }
> > >     val msg = s"Kill request for $driverId submitted"
> > >     logInfo(msg)
> > >     sender ! KillDriverResponse(true, msg)
> > >   case None =>
> > >     val msg = s"Could not find running driver $driverId"
> > >     logWarning(msg)
> > >     sender ! KillDriverResponse(false, msg)
> > > }
> > >
> > > ...using fold we end up with...
> > >
> > > driver.fold
> > >   {
> > >     val msg = s"Could not find running driver $driverId"
> > >     logWarning(msg)
> > >     sender ! KillDriverResponse(false, msg)
> > >   }
> > >   { d =>
> > >     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> > >     else {
> > >       d.worker.foreach { w =>
> > >         w.actor ! KillDriver(driverId)
> > >       }
> > >     }
> > >     val msg = s"Kill request for $driverId submitted"
> > >     logInfo(msg)
> > >     sender ! KillDriverResponse(true, msg)
> > >   }
> > >
> > >
> > > So the basic pattern (and my proposed formatting standard) for folding
> > over
> > > an `Option[A]` from which you need to produce a B (which may be Unit if
> > > you're only interested in side effects) is:
> > >
> > > anOption.fold
> > >   {
> > >     // something that evaluates to a B if anOption = None
> > >   }
> > >   { a =>
> > >     // something that transforms `a` into a B if anOption = Some(a)
> > >   }
> > >
> > >
> > > Any thoughts?  Does anyone really, really hate this style of coding and
> > > oppose its use in Spark?
> > >
> >
> >
> >
> > --
> > --
> > Evan Chan
> > Staff Engineer
> > [hidden email]  |
> >
> > <http://www.ooyala.com/>
> > <http://www.facebook.com/ooyala><http://www.linkedin.com/company/ooyala
> ><
> > http://www.twitter.com/ooyala>
> >
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Holden Karau
I personally with Evan in that I prefer map with getOrElse over fold with
options (but that just my personal preference) :)


On Thu, Dec 26, 2013 at 7:58 PM, Reynold Xin <[hidden email]> wrote:

> I'm not strongly against Option.fold, but I find the readability getting
> worse for the use case you brought up.  For the use case of if/else, I find
> Option.fold pretty confusing because it reverses the order of Some vs None.
> Also, when code gets long, the lack of an obvious boundary (the only
> boundary is "} {") with two closures is pretty confusing.
>
>
> On Thu, Dec 26, 2013 at 4:23 PM, Mark Hamstra <[hidden email]
> >wrote:
>
> > On the contrary, it is the completely natural place for the initial value
> > of the accumulator, and provides the expected result of folding over an
> > empty collection.
> >
> > scala> val l: List[Int] = List()
> >
> > l: List[Int] = List()
> >
> >
> > scala> l.fold(42)(_ + _)
> >
> > res0: Int = 42
> >
> >
> > scala> val o: Option[Int] = None
> >
> > o: Option[Int] = None
> >
> >
> > scala> o.fold(42)(_ + 1)
> >
> > res1: Int = 42
> >
> >
> > On Thu, Dec 26, 2013 at 5:51 PM, Evan Chan <[hidden email]> wrote:
> >
> > > +1 for using more functional idioms in general.
> > >
> > > That's a pretty clever use of `fold`, but putting the default condition
> > > first there makes it not as intuitive.   What about the following,
> which
> > > are more readable?
> > >
> > >     option.map { a => someFuncMakesB() }
> > >               .getOrElse(b)
> > >
> > >     option.map { a => someFuncMakesB() }
> > >               .orElse { a => otherDefaultB() }.get
> > >
> > >
> > > On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <
> [hidden email]
> > > >wrote:
> > >
> > > > In code added to Spark over the past several months, I'm glad to see
> > more
> > > > use of `foreach`, `for`, `map` and `flatMap` over `Option` instead of
> > > > pattern matching boilerplate.  There are opportunities to push
> `Option`
> > > > idioms even further now that we are using Scala 2.10 in master, but I
> > > want
> > > > to discuss the issue here a little bit before committing code whose
> > form
> > > > may be a little unfamiliar to some Spark developers.
> > > >
> > > > In particular, I really like the use of `fold` with `Option` to
> cleanly
> > > an
> > > > concisely express the "do something if the Option is None; do
> something
> > > > else with the thing contained in the Option if it is Some" code
> > fragment.
> > > >
> > > > An example:
> > > >
> > > > Instead of...
> > > >
> > > > val driver = drivers.find(_.id == driverId)
> > > > driver match {
> > > >   case Some(d) =>
> > > >     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> > > >     else {
> > > >       d.worker.foreach { w =>
> > > >         w.actor ! KillDriver(driverId)
> > > >       }
> > > >     }
> > > >     val msg = s"Kill request for $driverId submitted"
> > > >     logInfo(msg)
> > > >     sender ! KillDriverResponse(true, msg)
> > > >   case None =>
> > > >     val msg = s"Could not find running driver $driverId"
> > > >     logWarning(msg)
> > > >     sender ! KillDriverResponse(false, msg)
> > > > }
> > > >
> > > > ...using fold we end up with...
> > > >
> > > > driver.fold
> > > >   {
> > > >     val msg = s"Could not find running driver $driverId"
> > > >     logWarning(msg)
> > > >     sender ! KillDriverResponse(false, msg)
> > > >   }
> > > >   { d =>
> > > >     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> > > >     else {
> > > >       d.worker.foreach { w =>
> > > >         w.actor ! KillDriver(driverId)
> > > >       }
> > > >     }
> > > >     val msg = s"Kill request for $driverId submitted"
> > > >     logInfo(msg)
> > > >     sender ! KillDriverResponse(true, msg)
> > > >   }
> > > >
> > > >
> > > > So the basic pattern (and my proposed formatting standard) for
> folding
> > > over
> > > > an `Option[A]` from which you need to produce a B (which may be Unit
> if
> > > > you're only interested in side effects) is:
> > > >
> > > > anOption.fold
> > > >   {
> > > >     // something that evaluates to a B if anOption = None
> > > >   }
> > > >   { a =>
> > > >     // something that transforms `a` into a B if anOption = Some(a)
> > > >   }
> > > >
> > > >
> > > > Any thoughts?  Does anyone really, really hate this style of coding
> and
> > > > oppose its use in Spark?
> > > >
> > >
> > >
> > >
> > > --
> > > --
> > > Evan Chan
> > > Staff Engineer
> > > [hidden email]  |
> > >
> > > <http://www.ooyala.com/>
> > > <http://www.facebook.com/ooyala><
> http://www.linkedin.com/company/ooyala
> > ><
> > > http://www.twitter.com/ooyala>
> > >
> >
>



--
Cell : 425-233-8271
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Kay Ousterhout
I agree with what Reynold said -- there's not a big benefit in terms of
lines of code (esp. compared to using getOrElse) and I think it hurts code
readability.  One of the great things about the current Spark codebase is
that it's very accessible for newcomers -- something that would be less
true with this use of "fold".


On Thu, Dec 26, 2013 at 8:11 PM, Holden Karau <[hidden email]> wrote:

> I personally with Evan in that I prefer map with getOrElse over fold with
> options (but that just my personal preference) :)
>
>
> On Thu, Dec 26, 2013 at 7:58 PM, Reynold Xin <[hidden email]> wrote:
>
> > I'm not strongly against Option.fold, but I find the readability getting
> > worse for the use case you brought up.  For the use case of if/else, I
> find
> > Option.fold pretty confusing because it reverses the order of Some vs
> None.
> > Also, when code gets long, the lack of an obvious boundary (the only
> > boundary is "} {") with two closures is pretty confusing.
> >
> >
> > On Thu, Dec 26, 2013 at 4:23 PM, Mark Hamstra <[hidden email]
> > >wrote:
> >
> > > On the contrary, it is the completely natural place for the initial
> value
> > > of the accumulator, and provides the expected result of folding over an
> > > empty collection.
> > >
> > > scala> val l: List[Int] = List()
> > >
> > > l: List[Int] = List()
> > >
> > >
> > > scala> l.fold(42)(_ + _)
> > >
> > > res0: Int = 42
> > >
> > >
> > > scala> val o: Option[Int] = None
> > >
> > > o: Option[Int] = None
> > >
> > >
> > > scala> o.fold(42)(_ + 1)
> > >
> > > res1: Int = 42
> > >
> > >
> > > On Thu, Dec 26, 2013 at 5:51 PM, Evan Chan <[hidden email]> wrote:
> > >
> > > > +1 for using more functional idioms in general.
> > > >
> > > > That's a pretty clever use of `fold`, but putting the default
> condition
> > > > first there makes it not as intuitive.   What about the following,
> > which
> > > > are more readable?
> > > >
> > > >     option.map { a => someFuncMakesB() }
> > > >               .getOrElse(b)
> > > >
> > > >     option.map { a => someFuncMakesB() }
> > > >               .orElse { a => otherDefaultB() }.get
> > > >
> > > >
> > > > On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <
> > [hidden email]
> > > > >wrote:
> > > >
> > > > > In code added to Spark over the past several months, I'm glad to
> see
> > > more
> > > > > use of `foreach`, `for`, `map` and `flatMap` over `Option` instead
> of
> > > > > pattern matching boilerplate.  There are opportunities to push
> > `Option`
> > > > > idioms even further now that we are using Scala 2.10 in master,
> but I
> > > > want
> > > > > to discuss the issue here a little bit before committing code whose
> > > form
> > > > > may be a little unfamiliar to some Spark developers.
> > > > >
> > > > > In particular, I really like the use of `fold` with `Option` to
> > cleanly
> > > > an
> > > > > concisely express the "do something if the Option is None; do
> > something
> > > > > else with the thing contained in the Option if it is Some" code
> > > fragment.
> > > > >
> > > > > An example:
> > > > >
> > > > > Instead of...
> > > > >
> > > > > val driver = drivers.find(_.id == driverId)
> > > > > driver match {
> > > > >   case Some(d) =>
> > > > >     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> > > > >     else {
> > > > >       d.worker.foreach { w =>
> > > > >         w.actor ! KillDriver(driverId)
> > > > >       }
> > > > >     }
> > > > >     val msg = s"Kill request for $driverId submitted"
> > > > >     logInfo(msg)
> > > > >     sender ! KillDriverResponse(true, msg)
> > > > >   case None =>
> > > > >     val msg = s"Could not find running driver $driverId"
> > > > >     logWarning(msg)
> > > > >     sender ! KillDriverResponse(false, msg)
> > > > > }
> > > > >
> > > > > ...using fold we end up with...
> > > > >
> > > > > driver.fold
> > > > >   {
> > > > >     val msg = s"Could not find running driver $driverId"
> > > > >     logWarning(msg)
> > > > >     sender ! KillDriverResponse(false, msg)
> > > > >   }
> > > > >   { d =>
> > > > >     if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> > > > >     else {
> > > > >       d.worker.foreach { w =>
> > > > >         w.actor ! KillDriver(driverId)
> > > > >       }
> > > > >     }
> > > > >     val msg = s"Kill request for $driverId submitted"
> > > > >     logInfo(msg)
> > > > >     sender ! KillDriverResponse(true, msg)
> > > > >   }
> > > > >
> > > > >
> > > > > So the basic pattern (and my proposed formatting standard) for
> > folding
> > > > over
> > > > > an `Option[A]` from which you need to produce a B (which may be
> Unit
> > if
> > > > > you're only interested in side effects) is:
> > > > >
> > > > > anOption.fold
> > > > >   {
> > > > >     // something that evaluates to a B if anOption = None
> > > > >   }
> > > > >   { a =>
> > > > >     // something that transforms `a` into a B if anOption = Some(a)
> > > > >   }
> > > > >
> > > > >
> > > > > Any thoughts?  Does anyone really, really hate this style of coding
> > and
> > > > > oppose its use in Spark?
> > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > --
> > > > Evan Chan
> > > > Staff Engineer
> > > > [hidden email]  |
> > > >
> > > > <http://www.ooyala.com/>
> > > > <http://www.facebook.com/ooyala><
> > http://www.linkedin.com/company/ooyala
> > > ><
> > > > http://www.twitter.com/ooyala>
> > > >
> > >
> >
>
>
>
> --
> Cell : 425-233-8271
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Matei Zaharia
Administrator
I agree about using getOrElse instead. In choosing which code style and idioms to use, my goal has always been to maximize the ease of *other developers* understanding the code, and most developers today still don’t know Scala. It’s fine to use a maps or matches, because their meaning is obvious, but fold on Option is not obvious (even foreach is kind of weird for new people). In this case the benefit is so small that it doesn’t seem worth it.

Note that if you use getOrElse, you can even throw exceptions in the “else” part if you’d like. (This is because Nothing is a subtype of every type in Scala.) So for example you can do val stuff = option.getOrElse(throw new Exception(“It wasn’t set”)). It looks a little weird, but note how the meaning is obvious even if you don’t know anything about the type system.

Matei

On Dec 27, 2013, at 12:12 AM, Kay Ousterhout <[hidden email]> wrote:

> I agree with what Reynold said -- there's not a big benefit in terms of
> lines of code (esp. compared to using getOrElse) and I think it hurts code
> readability.  One of the great things about the current Spark codebase is
> that it's very accessible for newcomers -- something that would be less
> true with this use of "fold".
>
>
> On Thu, Dec 26, 2013 at 8:11 PM, Holden Karau <[hidden email]> wrote:
>
>> I personally with Evan in that I prefer map with getOrElse over fold with
>> options (but that just my personal preference) :)
>>
>>
>> On Thu, Dec 26, 2013 at 7:58 PM, Reynold Xin <[hidden email]> wrote:
>>
>>> I'm not strongly against Option.fold, but I find the readability getting
>>> worse for the use case you brought up.  For the use case of if/else, I
>> find
>>> Option.fold pretty confusing because it reverses the order of Some vs
>> None.
>>> Also, when code gets long, the lack of an obvious boundary (the only
>>> boundary is "} {") with two closures is pretty confusing.
>>>
>>>
>>> On Thu, Dec 26, 2013 at 4:23 PM, Mark Hamstra <[hidden email]
>>>> wrote:
>>>
>>>> On the contrary, it is the completely natural place for the initial
>> value
>>>> of the accumulator, and provides the expected result of folding over an
>>>> empty collection.
>>>>
>>>> scala> val l: List[Int] = List()
>>>>
>>>> l: List[Int] = List()
>>>>
>>>>
>>>> scala> l.fold(42)(_ + _)
>>>>
>>>> res0: Int = 42
>>>>
>>>>
>>>> scala> val o: Option[Int] = None
>>>>
>>>> o: Option[Int] = None
>>>>
>>>>
>>>> scala> o.fold(42)(_ + 1)
>>>>
>>>> res1: Int = 42
>>>>
>>>>
>>>> On Thu, Dec 26, 2013 at 5:51 PM, Evan Chan <[hidden email]> wrote:
>>>>
>>>>> +1 for using more functional idioms in general.
>>>>>
>>>>> That's a pretty clever use of `fold`, but putting the default
>> condition
>>>>> first there makes it not as intuitive.   What about the following,
>>> which
>>>>> are more readable?
>>>>>
>>>>>    option.map { a => someFuncMakesB() }
>>>>>              .getOrElse(b)
>>>>>
>>>>>    option.map { a => someFuncMakesB() }
>>>>>              .orElse { a => otherDefaultB() }.get
>>>>>
>>>>>
>>>>> On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <
>>> [hidden email]
>>>>>> wrote:
>>>>>
>>>>>> In code added to Spark over the past several months, I'm glad to
>> see
>>>> more
>>>>>> use of `foreach`, `for`, `map` and `flatMap` over `Option` instead
>> of
>>>>>> pattern matching boilerplate.  There are opportunities to push
>>> `Option`
>>>>>> idioms even further now that we are using Scala 2.10 in master,
>> but I
>>>>> want
>>>>>> to discuss the issue here a little bit before committing code whose
>>>> form
>>>>>> may be a little unfamiliar to some Spark developers.
>>>>>>
>>>>>> In particular, I really like the use of `fold` with `Option` to
>>> cleanly
>>>>> an
>>>>>> concisely express the "do something if the Option is None; do
>>> something
>>>>>> else with the thing contained in the Option if it is Some" code
>>>> fragment.
>>>>>>
>>>>>> An example:
>>>>>>
>>>>>> Instead of...
>>>>>>
>>>>>> val driver = drivers.find(_.id == driverId)
>>>>>> driver match {
>>>>>>  case Some(d) =>
>>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
>>>>>>    else {
>>>>>>      d.worker.foreach { w =>
>>>>>>        w.actor ! KillDriver(driverId)
>>>>>>      }
>>>>>>    }
>>>>>>    val msg = s"Kill request for $driverId submitted"
>>>>>>    logInfo(msg)
>>>>>>    sender ! KillDriverResponse(true, msg)
>>>>>>  case None =>
>>>>>>    val msg = s"Could not find running driver $driverId"
>>>>>>    logWarning(msg)
>>>>>>    sender ! KillDriverResponse(false, msg)
>>>>>> }
>>>>>>
>>>>>> ...using fold we end up with...
>>>>>>
>>>>>> driver.fold
>>>>>>  {
>>>>>>    val msg = s"Could not find running driver $driverId"
>>>>>>    logWarning(msg)
>>>>>>    sender ! KillDriverResponse(false, msg)
>>>>>>  }
>>>>>>  { d =>
>>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
>>>>>>    else {
>>>>>>      d.worker.foreach { w =>
>>>>>>        w.actor ! KillDriver(driverId)
>>>>>>      }
>>>>>>    }
>>>>>>    val msg = s"Kill request for $driverId submitted"
>>>>>>    logInfo(msg)
>>>>>>    sender ! KillDriverResponse(true, msg)
>>>>>>  }
>>>>>>
>>>>>>
>>>>>> So the basic pattern (and my proposed formatting standard) for
>>> folding
>>>>> over
>>>>>> an `Option[A]` from which you need to produce a B (which may be
>> Unit
>>> if
>>>>>> you're only interested in side effects) is:
>>>>>>
>>>>>> anOption.fold
>>>>>>  {
>>>>>>    // something that evaluates to a B if anOption = None
>>>>>>  }
>>>>>>  { a =>
>>>>>>    // something that transforms `a` into a B if anOption = Some(a)
>>>>>>  }
>>>>>>
>>>>>>
>>>>>> Any thoughts?  Does anyone really, really hate this style of coding
>>> and
>>>>>> oppose its use in Spark?
>>>>>>
>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> --
>>>>> Evan Chan
>>>>> Staff Engineer
>>>>> [hidden email]  |
>>>>>
>>>>> <http://www.ooyala.com/>
>>>>> <http://www.facebook.com/ooyala><
>>> http://www.linkedin.com/company/ooyala
>>>>> <
>>>>> http://www.twitter.com/ooyala>
>>>>>
>>>>
>>>
>>
>>
>>
>> --
>> Cell : 425-233-8271
>>

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Nick Pentreath
+1 for getOrElse


When I was new to Scala I tended to use match almost like if/else statements with Option. These days I try to use map/flatMap instead and use getOrElse extensively and I for one find it very intuitive.




I also agree that the fold syntax seems way less intuitive and I certainly prefer readable Scala code to that which might be more "idiomatic" but which I honestly tend to find very inscrutable and hard to grok quickly.

Sent from Mailbox for iPhone

On Fri, Dec 27, 2013 at 9:06 AM, Matei Zaharia <[hidden email]>
wrote:

> I agree about using getOrElse instead. In choosing which code style and idioms to use, my goal has always been to maximize the ease of *other developers* understanding the code, and most developers today still don’t know Scala. It’s fine to use a maps or matches, because their meaning is obvious, but fold on Option is not obvious (even foreach is kind of weird for new people). In this case the benefit is so small that it doesn’t seem worth it.
> Note that if you use getOrElse, you can even throw exceptions in the “else” part if you’d like. (This is because Nothing is a subtype of every type in Scala.) So for example you can do val stuff = option.getOrElse(throw new Exception(“It wasn’t set”)). It looks a little weird, but note how the meaning is obvious even if you don’t know anything about the type system.
> Matei
> On Dec 27, 2013, at 12:12 AM, Kay Ousterhout <[hidden email]> wrote:
>> I agree with what Reynold said -- there's not a big benefit in terms of
>> lines of code (esp. compared to using getOrElse) and I think it hurts code
>> readability.  One of the great things about the current Spark codebase is
>> that it's very accessible for newcomers -- something that would be less
>> true with this use of "fold".
>>
>>
>> On Thu, Dec 26, 2013 at 8:11 PM, Holden Karau <[hidden email]> wrote:
>>
>>> I personally with Evan in that I prefer map with getOrElse over fold with
>>> options (but that just my personal preference) :)
>>>
>>>
>>> On Thu, Dec 26, 2013 at 7:58 PM, Reynold Xin <[hidden email]> wrote:
>>>
>>>> I'm not strongly against Option.fold, but I find the readability getting
>>>> worse for the use case you brought up.  For the use case of if/else, I
>>> find
>>>> Option.fold pretty confusing because it reverses the order of Some vs
>>> None.
>>>> Also, when code gets long, the lack of an obvious boundary (the only
>>>> boundary is "} {") with two closures is pretty confusing.
>>>>
>>>>
>>>> On Thu, Dec 26, 2013 at 4:23 PM, Mark Hamstra <[hidden email]
>>>>> wrote:
>>>>
>>>>> On the contrary, it is the completely natural place for the initial
>>> value
>>>>> of the accumulator, and provides the expected result of folding over an
>>>>> empty collection.
>>>>>
>>>>> scala> val l: List[Int] = List()
>>>>>
>>>>> l: List[Int] = List()
>>>>>
>>>>>
>>>>> scala> l.fold(42)(_ + _)
>>>>>
>>>>> res0: Int = 42
>>>>>
>>>>>
>>>>> scala> val o: Option[Int] = None
>>>>>
>>>>> o: Option[Int] = None
>>>>>
>>>>>
>>>>> scala> o.fold(42)(_ + 1)
>>>>>
>>>>> res1: Int = 42
>>>>>
>>>>>
>>>>> On Thu, Dec 26, 2013 at 5:51 PM, Evan Chan <[hidden email]> wrote:
>>>>>
>>>>>> +1 for using more functional idioms in general.
>>>>>>
>>>>>> That's a pretty clever use of `fold`, but putting the default
>>> condition
>>>>>> first there makes it not as intuitive.   What about the following,
>>>> which
>>>>>> are more readable?
>>>>>>
>>>>>>    option.map { a => someFuncMakesB() }
>>>>>>              .getOrElse(b)
>>>>>>
>>>>>>    option.map { a => someFuncMakesB() }
>>>>>>              .orElse { a => otherDefaultB() }.get
>>>>>>
>>>>>>
>>>>>> On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <
>>>> [hidden email]
>>>>>>> wrote:
>>>>>>
>>>>>>> In code added to Spark over the past several months, I'm glad to
>>> see
>>>>> more
>>>>>>> use of `foreach`, `for`, `map` and `flatMap` over `Option` instead
>>> of
>>>>>>> pattern matching boilerplate.  There are opportunities to push
>>>> `Option`
>>>>>>> idioms even further now that we are using Scala 2.10 in master,
>>> but I
>>>>>> want
>>>>>>> to discuss the issue here a little bit before committing code whose
>>>>> form
>>>>>>> may be a little unfamiliar to some Spark developers.
>>>>>>>
>>>>>>> In particular, I really like the use of `fold` with `Option` to
>>>> cleanly
>>>>>> an
>>>>>>> concisely express the "do something if the Option is None; do
>>>> something
>>>>>>> else with the thing contained in the Option if it is Some" code
>>>>> fragment.
>>>>>>>
>>>>>>> An example:
>>>>>>>
>>>>>>> Instead of...
>>>>>>>
>>>>>>> val driver = drivers.find(_.id == driverId)
>>>>>>> driver match {
>>>>>>>  case Some(d) =>
>>>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
>>>>>>>    else {
>>>>>>>      d.worker.foreach { w =>
>>>>>>>        w.actor ! KillDriver(driverId)
>>>>>>>      }
>>>>>>>    }
>>>>>>>    val msg = s"Kill request for $driverId submitted"
>>>>>>>    logInfo(msg)
>>>>>>>    sender ! KillDriverResponse(true, msg)
>>>>>>>  case None =>
>>>>>>>    val msg = s"Could not find running driver $driverId"
>>>>>>>    logWarning(msg)
>>>>>>>    sender ! KillDriverResponse(false, msg)
>>>>>>> }
>>>>>>>
>>>>>>> ...using fold we end up with...
>>>>>>>
>>>>>>> driver.fold
>>>>>>>  {
>>>>>>>    val msg = s"Could not find running driver $driverId"
>>>>>>>    logWarning(msg)
>>>>>>>    sender ! KillDriverResponse(false, msg)
>>>>>>>  }
>>>>>>>  { d =>
>>>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
>>>>>>>    else {
>>>>>>>      d.worker.foreach { w =>
>>>>>>>        w.actor ! KillDriver(driverId)
>>>>>>>      }
>>>>>>>    }
>>>>>>>    val msg = s"Kill request for $driverId submitted"
>>>>>>>    logInfo(msg)
>>>>>>>    sender ! KillDriverResponse(true, msg)
>>>>>>>  }
>>>>>>>
>>>>>>>
>>>>>>> So the basic pattern (and my proposed formatting standard) for
>>>> folding
>>>>>> over
>>>>>>> an `Option[A]` from which you need to produce a B (which may be
>>> Unit
>>>> if
>>>>>>> you're only interested in side effects) is:
>>>>>>>
>>>>>>> anOption.fold
>>>>>>>  {
>>>>>>>    // something that evaluates to a B if anOption = None
>>>>>>>  }
>>>>>>>  { a =>
>>>>>>>    // something that transforms `a` into a B if anOption = Some(a)
>>>>>>>  }
>>>>>>>
>>>>>>>
>>>>>>> Any thoughts?  Does anyone really, really hate this style of coding
>>>> and
>>>>>>> oppose its use in Spark?
>>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> --
>>>>>> --
>>>>>> Evan Chan
>>>>>> Staff Engineer
>>>>>> [hidden email]  |
>>>>>>
>>>>>> <http://www.ooyala.com/>
>>>>>> <http://www.facebook.com/ooyala><
>>>> http://www.linkedin.com/company/ooyala
>>>>>> <
>>>>>> http://www.twitter.com/ooyala>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>> --
>>> Cell : 425-233-8271
>>>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Mark Hamstra
In reply to this post by Matei Zaharia
Well, you can throw exceptions from within a fold as well:

scala> val o: Option[Int] = None
o: Option[Int] = None

scala> o.fold { throw new Exception("It wasn't set") } { _.toString }
java.lang.Exception: It wasn't set
  at $anonfun$1.apply(<console>:9)
  at $anonfun$1.apply(<console>:9)
  at scala.Option.fold(Option.scala:157)
  ... 32 elided

But it looks to me like there's an emerging majority, if not a consensus,
who find clearly expressing the catamorphism not to be very useful or
helpful on its own, so I can certainly make do with other syntactic
constructs that others appreciate more.  That was worth clarifying for me,
so thank you all.



On Thu, Dec 26, 2013 at 11:06 PM, Matei Zaharia <[hidden email]>wrote:

> I agree about using getOrElse instead. In choosing which code style and
> idioms to use, my goal has always been to maximize the ease of *other
> developers* understanding the code, and most developers today still don’t
> know Scala. It’s fine to use a maps or matches, because their meaning is
> obvious, but fold on Option is not obvious (even foreach is kind of weird
> for new people). In this case the benefit is so small that it doesn’t seem
> worth it.
>
> Note that if you use getOrElse, you can even throw exceptions in the
> “else” part if you’d like. (This is because Nothing is a subtype of every
> type in Scala.) So for example you can do val stuff =
> option.getOrElse(throw new Exception(“It wasn’t set”)). It looks a little
> weird, but note how the meaning is obvious even if you don’t know anything
> about the type system.
>
> Matei
>
> On Dec 27, 2013, at 12:12 AM, Kay Ousterhout <[hidden email]>
> wrote:
>
> > I agree with what Reynold said -- there's not a big benefit in terms of
> > lines of code (esp. compared to using getOrElse) and I think it hurts
> code
> > readability.  One of the great things about the current Spark codebase is
> > that it's very accessible for newcomers -- something that would be less
> > true with this use of "fold".
> >
> >
> > On Thu, Dec 26, 2013 at 8:11 PM, Holden Karau <[hidden email]>
> wrote:
> >
> >> I personally with Evan in that I prefer map with getOrElse over fold
> with
> >> options (but that just my personal preference) :)
> >>
> >>
> >> On Thu, Dec 26, 2013 at 7:58 PM, Reynold Xin <[hidden email]>
> wrote:
> >>
> >>> I'm not strongly against Option.fold, but I find the readability
> getting
> >>> worse for the use case you brought up.  For the use case of if/else, I
> >> find
> >>> Option.fold pretty confusing because it reverses the order of Some vs
> >> None.
> >>> Also, when code gets long, the lack of an obvious boundary (the only
> >>> boundary is "} {") with two closures is pretty confusing.
> >>>
> >>>
> >>> On Thu, Dec 26, 2013 at 4:23 PM, Mark Hamstra <[hidden email]
> >>>> wrote:
> >>>
> >>>> On the contrary, it is the completely natural place for the initial
> >> value
> >>>> of the accumulator, and provides the expected result of folding over
> an
> >>>> empty collection.
> >>>>
> >>>> scala> val l: List[Int] = List()
> >>>>
> >>>> l: List[Int] = List()
> >>>>
> >>>>
> >>>> scala> l.fold(42)(_ + _)
> >>>>
> >>>> res0: Int = 42
> >>>>
> >>>>
> >>>> scala> val o: Option[Int] = None
> >>>>
> >>>> o: Option[Int] = None
> >>>>
> >>>>
> >>>> scala> o.fold(42)(_ + 1)
> >>>>
> >>>> res1: Int = 42
> >>>>
> >>>>
> >>>> On Thu, Dec 26, 2013 at 5:51 PM, Evan Chan <[hidden email]> wrote:
> >>>>
> >>>>> +1 for using more functional idioms in general.
> >>>>>
> >>>>> That's a pretty clever use of `fold`, but putting the default
> >> condition
> >>>>> first there makes it not as intuitive.   What about the following,
> >>> which
> >>>>> are more readable?
> >>>>>
> >>>>>    option.map { a => someFuncMakesB() }
> >>>>>              .getOrElse(b)
> >>>>>
> >>>>>    option.map { a => someFuncMakesB() }
> >>>>>              .orElse { a => otherDefaultB() }.get
> >>>>>
> >>>>>
> >>>>> On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <
> >>> [hidden email]
> >>>>>> wrote:
> >>>>>
> >>>>>> In code added to Spark over the past several months, I'm glad to
> >> see
> >>>> more
> >>>>>> use of `foreach`, `for`, `map` and `flatMap` over `Option` instead
> >> of
> >>>>>> pattern matching boilerplate.  There are opportunities to push
> >>> `Option`
> >>>>>> idioms even further now that we are using Scala 2.10 in master,
> >> but I
> >>>>> want
> >>>>>> to discuss the issue here a little bit before committing code whose
> >>>> form
> >>>>>> may be a little unfamiliar to some Spark developers.
> >>>>>>
> >>>>>> In particular, I really like the use of `fold` with `Option` to
> >>> cleanly
> >>>>> an
> >>>>>> concisely express the "do something if the Option is None; do
> >>> something
> >>>>>> else with the thing contained in the Option if it is Some" code
> >>>> fragment.
> >>>>>>
> >>>>>> An example:
> >>>>>>
> >>>>>> Instead of...
> >>>>>>
> >>>>>> val driver = drivers.find(_.id == driverId)
> >>>>>> driver match {
> >>>>>>  case Some(d) =>
> >>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> >>>>>>    else {
> >>>>>>      d.worker.foreach { w =>
> >>>>>>        w.actor ! KillDriver(driverId)
> >>>>>>      }
> >>>>>>    }
> >>>>>>    val msg = s"Kill request for $driverId submitted"
> >>>>>>    logInfo(msg)
> >>>>>>    sender ! KillDriverResponse(true, msg)
> >>>>>>  case None =>
> >>>>>>    val msg = s"Could not find running driver $driverId"
> >>>>>>    logWarning(msg)
> >>>>>>    sender ! KillDriverResponse(false, msg)
> >>>>>> }
> >>>>>>
> >>>>>> ...using fold we end up with...
> >>>>>>
> >>>>>> driver.fold
> >>>>>>  {
> >>>>>>    val msg = s"Could not find running driver $driverId"
> >>>>>>    logWarning(msg)
> >>>>>>    sender ! KillDriverResponse(false, msg)
> >>>>>>  }
> >>>>>>  { d =>
> >>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> >>>>>>    else {
> >>>>>>      d.worker.foreach { w =>
> >>>>>>        w.actor ! KillDriver(driverId)
> >>>>>>      }
> >>>>>>    }
> >>>>>>    val msg = s"Kill request for $driverId submitted"
> >>>>>>    logInfo(msg)
> >>>>>>    sender ! KillDriverResponse(true, msg)
> >>>>>>  }
> >>>>>>
> >>>>>>
> >>>>>> So the basic pattern (and my proposed formatting standard) for
> >>> folding
> >>>>> over
> >>>>>> an `Option[A]` from which you need to produce a B (which may be
> >> Unit
> >>> if
> >>>>>> you're only interested in side effects) is:
> >>>>>>
> >>>>>> anOption.fold
> >>>>>>  {
> >>>>>>    // something that evaluates to a B if anOption = None
> >>>>>>  }
> >>>>>>  { a =>
> >>>>>>    // something that transforms `a` into a B if anOption = Some(a)
> >>>>>>  }
> >>>>>>
> >>>>>>
> >>>>>> Any thoughts?  Does anyone really, really hate this style of coding
> >>> and
> >>>>>> oppose its use in Spark?
> >>>>>>
> >>>>>
> >>>>>
> >>>>>
> >>>>> --
> >>>>> --
> >>>>> Evan Chan
> >>>>> Staff Engineer
> >>>>> [hidden email]  |
> >>>>>
> >>>>> <http://www.ooyala.com/>
> >>>>> <http://www.facebook.com/ooyala><
> >>> http://www.linkedin.com/company/ooyala
> >>>>> <
> >>>>> http://www.twitter.com/ooyala>
> >>>>>
> >>>>
> >>>
> >>
> >>
> >>
> >> --
> >> Cell : 425-233-8271
> >>
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Christopher Nguyen
In reply to this post by Nick Pentreath
I've learned and unlearned enough things to be careful when claiming
something is "more intuitive" than another, since it's subject to prior
knowledge. When I first encountered map().getOrElse() it wasn't any more
intuitive than this fold()() syntax. Maybe the "OrElse" helps a bit, but
the "get" in front of it confuses matters again (it sets one up to expect
two things following, not one). Meanwhile, people coming from
data-structure-folding background would argue that fold()() is more
"intuitive".

If the choice is among three alternatives (match, map().getOrElse(), and
fold()()), and the goal is intuitively obvious syntax to the broadest
audience, then "match" wins by a reasonably good distance, with the latter
two about equal. This tie could be broken by the fact that more people by
now know about getOrElse than fold, crossed with the fact that it probably
isn't on the top of the Spark community's agenda to be avant garde on new
Scala syntax.



--
Christopher T. Nguyen
Co-founder & CEO, Adatao <http://adatao.com>
linkedin.com/in/ctnguyen



On Thu, Dec 26, 2013 at 11:40 PM, Nick Pentreath
<[hidden email]>wrote:

> +1 for getOrElse
>
>
> When I was new to Scala I tended to use match almost like if/else
> statements with Option. These days I try to use map/flatMap instead and use
> getOrElse extensively and I for one find it very intuitive.
>
>
>
>
> I also agree that the fold syntax seems way less intuitive and I certainly
> prefer readable Scala code to that which might be more "idiomatic" but
> which I honestly tend to find very inscrutable and hard to grok quickly.
> —
> Sent from Mailbox for iPhone
>
> On Fri, Dec 27, 2013 at 9:06 AM, Matei Zaharia <[hidden email]>
> wrote:
>
> > I agree about using getOrElse instead. In choosing which code style and
> idioms to use, my goal has always been to maximize the ease of *other
> developers* understanding the code, and most developers today still don’t
> know Scala. It’s fine to use a maps or matches, because their meaning is
> obvious, but fold on Option is not obvious (even foreach is kind of weird
> for new people). In this case the benefit is so small that it doesn’t seem
> worth it.
> > Note that if you use getOrElse, you can even throw exceptions in the
> “else” part if you’d like. (This is because Nothing is a subtype of every
> type in Scala.) So for example you can do val stuff =
> option.getOrElse(throw new Exception(“It wasn’t set”)). It looks a little
> weird, but note how the meaning is obvious even if you don’t know anything
> about the type system.
> > Matei
> > On Dec 27, 2013, at 12:12 AM, Kay Ousterhout <[hidden email]>
> wrote:
> >> I agree with what Reynold said -- there's not a big benefit in terms of
> >> lines of code (esp. compared to using getOrElse) and I think it hurts
> code
> >> readability.  One of the great things about the current Spark codebase
> is
> >> that it's very accessible for newcomers -- something that would be less
> >> true with this use of "fold".
> >>
> >>
> >> On Thu, Dec 26, 2013 at 8:11 PM, Holden Karau <[hidden email]>
> wrote:
> >>
> >>> I personally with Evan in that I prefer map with getOrElse over fold
> with
> >>> options (but that just my personal preference) :)
> >>>
> >>>
> >>> On Thu, Dec 26, 2013 at 7:58 PM, Reynold Xin <[hidden email]>
> wrote:
> >>>
> >>>> I'm not strongly against Option.fold, but I find the readability
> getting
> >>>> worse for the use case you brought up.  For the use case of if/else, I
> >>> find
> >>>> Option.fold pretty confusing because it reverses the order of Some vs
> >>> None.
> >>>> Also, when code gets long, the lack of an obvious boundary (the only
> >>>> boundary is "} {") with two closures is pretty confusing.
> >>>>
> >>>>
> >>>> On Thu, Dec 26, 2013 at 4:23 PM, Mark Hamstra <
> [hidden email]
> >>>>> wrote:
> >>>>
> >>>>> On the contrary, it is the completely natural place for the initial
> >>> value
> >>>>> of the accumulator, and provides the expected result of folding over
> an
> >>>>> empty collection.
> >>>>>
> >>>>> scala> val l: List[Int] = List()
> >>>>>
> >>>>> l: List[Int] = List()
> >>>>>
> >>>>>
> >>>>> scala> l.fold(42)(_ + _)
> >>>>>
> >>>>> res0: Int = 42
> >>>>>
> >>>>>
> >>>>> scala> val o: Option[Int] = None
> >>>>>
> >>>>> o: Option[Int] = None
> >>>>>
> >>>>>
> >>>>> scala> o.fold(42)(_ + 1)
> >>>>>
> >>>>> res1: Int = 42
> >>>>>
> >>>>>
> >>>>> On Thu, Dec 26, 2013 at 5:51 PM, Evan Chan <[hidden email]> wrote:
> >>>>>
> >>>>>> +1 for using more functional idioms in general.
> >>>>>>
> >>>>>> That's a pretty clever use of `fold`, but putting the default
> >>> condition
> >>>>>> first there makes it not as intuitive.   What about the following,
> >>>> which
> >>>>>> are more readable?
> >>>>>>
> >>>>>>    option.map { a => someFuncMakesB() }
> >>>>>>              .getOrElse(b)
> >>>>>>
> >>>>>>    option.map { a => someFuncMakesB() }
> >>>>>>              .orElse { a => otherDefaultB() }.get
> >>>>>>
> >>>>>>
> >>>>>> On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <
> >>>> [hidden email]
> >>>>>>> wrote:
> >>>>>>
> >>>>>>> In code added to Spark over the past several months, I'm glad to
> >>> see
> >>>>> more
> >>>>>>> use of `foreach`, `for`, `map` and `flatMap` over `Option` instead
> >>> of
> >>>>>>> pattern matching boilerplate.  There are opportunities to push
> >>>> `Option`
> >>>>>>> idioms even further now that we are using Scala 2.10 in master,
> >>> but I
> >>>>>> want
> >>>>>>> to discuss the issue here a little bit before committing code whose
> >>>>> form
> >>>>>>> may be a little unfamiliar to some Spark developers.
> >>>>>>>
> >>>>>>> In particular, I really like the use of `fold` with `Option` to
> >>>> cleanly
> >>>>>> an
> >>>>>>> concisely express the "do something if the Option is None; do
> >>>> something
> >>>>>>> else with the thing contained in the Option if it is Some" code
> >>>>> fragment.
> >>>>>>>
> >>>>>>> An example:
> >>>>>>>
> >>>>>>> Instead of...
> >>>>>>>
> >>>>>>> val driver = drivers.find(_.id == driverId)
> >>>>>>> driver match {
> >>>>>>>  case Some(d) =>
> >>>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> >>>>>>>    else {
> >>>>>>>      d.worker.foreach { w =>
> >>>>>>>        w.actor ! KillDriver(driverId)
> >>>>>>>      }
> >>>>>>>    }
> >>>>>>>    val msg = s"Kill request for $driverId submitted"
> >>>>>>>    logInfo(msg)
> >>>>>>>    sender ! KillDriverResponse(true, msg)
> >>>>>>>  case None =>
> >>>>>>>    val msg = s"Could not find running driver $driverId"
> >>>>>>>    logWarning(msg)
> >>>>>>>    sender ! KillDriverResponse(false, msg)
> >>>>>>> }
> >>>>>>>
> >>>>>>> ...using fold we end up with...
> >>>>>>>
> >>>>>>> driver.fold
> >>>>>>>  {
> >>>>>>>    val msg = s"Could not find running driver $driverId"
> >>>>>>>    logWarning(msg)
> >>>>>>>    sender ! KillDriverResponse(false, msg)
> >>>>>>>  }
> >>>>>>>  { d =>
> >>>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> >>>>>>>    else {
> >>>>>>>      d.worker.foreach { w =>
> >>>>>>>        w.actor ! KillDriver(driverId)
> >>>>>>>      }
> >>>>>>>    }
> >>>>>>>    val msg = s"Kill request for $driverId submitted"
> >>>>>>>    logInfo(msg)
> >>>>>>>    sender ! KillDriverResponse(true, msg)
> >>>>>>>  }
> >>>>>>>
> >>>>>>>
> >>>>>>> So the basic pattern (and my proposed formatting standard) for
> >>>> folding
> >>>>>> over
> >>>>>>> an `Option[A]` from which you need to produce a B (which may be
> >>> Unit
> >>>> if
> >>>>>>> you're only interested in side effects) is:
> >>>>>>>
> >>>>>>> anOption.fold
> >>>>>>>  {
> >>>>>>>    // something that evaluates to a B if anOption = None
> >>>>>>>  }
> >>>>>>>  { a =>
> >>>>>>>    // something that transforms `a` into a B if anOption = Some(a)
> >>>>>>>  }
> >>>>>>>
> >>>>>>>
> >>>>>>> Any thoughts?  Does anyone really, really hate this style of coding
> >>>> and
> >>>>>>> oppose its use in Spark?
> >>>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> --
> >>>>>> Evan Chan
> >>>>>> Staff Engineer
> >>>>>> [hidden email]  |
> >>>>>>
> >>>>>> <http://www.ooyala.com/>
> >>>>>> <http://www.facebook.com/ooyala><
> >>>> http://www.linkedin.com/company/ooyala
> >>>>>> <
> >>>>>> http://www.twitter.com/ooyala>
> >>>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >>>
> >>> --
> >>> Cell : 425-233-8271
> >>>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: Option folding idiom

Imran Rashid
I'm also against option.fold (though I wouldn't say I "really, really hate
this style of coding"), for the readability reasons already mentioned.

I actually find myself pulling back from some scala-isms after having spent
some time with them, for readability / maintainability.




On Fri, Dec 27, 2013 at 2:57 AM, Christopher Nguyen <[hidden email]> wrote:

> I've learned and unlearned enough things to be careful when claiming
> something is "more intuitive" than another, since it's subject to prior
> knowledge. When I first encountered map().getOrElse() it wasn't any more
> intuitive than this fold()() syntax. Maybe the "OrElse" helps a bit, but
> the "get" in front of it confuses matters again (it sets one up to expect
> two things following, not one). Meanwhile, people coming from
> data-structure-folding background would argue that fold()() is more
> "intuitive".
>
> If the choice is among three alternatives (match, map().getOrElse(), and
> fold()()), and the goal is intuitively obvious syntax to the broadest
> audience, then "match" wins by a reasonably good distance, with the latter
> two about equal. This tie could be broken by the fact that more people by
> now know about getOrElse than fold, crossed with the fact that it probably
> isn't on the top of the Spark community's agenda to be avant garde on new
> Scala syntax.
>
>
>
> --
> Christopher T. Nguyen
> Co-founder & CEO, Adatao <http://adatao.com>
> linkedin.com/in/ctnguyen
>
>
>
> On Thu, Dec 26, 2013 at 11:40 PM, Nick Pentreath
> <[hidden email]>wrote:
>
> > +1 for getOrElse
> >
> >
> > When I was new to Scala I tended to use match almost like if/else
> > statements with Option. These days I try to use map/flatMap instead and
> use
> > getOrElse extensively and I for one find it very intuitive.
> >
> >
> >
> >
> > I also agree that the fold syntax seems way less intuitive and I
> certainly
> > prefer readable Scala code to that which might be more "idiomatic" but
> > which I honestly tend to find very inscrutable and hard to grok quickly.
> > —
> > Sent from Mailbox for iPhone
> >
> > On Fri, Dec 27, 2013 at 9:06 AM, Matei Zaharia <[hidden email]>
> > wrote:
> >
> > > I agree about using getOrElse instead. In choosing which code style and
> > idioms to use, my goal has always been to maximize the ease of *other
> > developers* understanding the code, and most developers today still don’t
> > know Scala. It’s fine to use a maps or matches, because their meaning is
> > obvious, but fold on Option is not obvious (even foreach is kind of weird
> > for new people). In this case the benefit is so small that it doesn’t
> seem
> > worth it.
> > > Note that if you use getOrElse, you can even throw exceptions in the
> > “else” part if you’d like. (This is because Nothing is a subtype of every
> > type in Scala.) So for example you can do val stuff =
> > option.getOrElse(throw new Exception(“It wasn’t set”)). It looks a little
> > weird, but note how the meaning is obvious even if you don’t know
> anything
> > about the type system.
> > > Matei
> > > On Dec 27, 2013, at 12:12 AM, Kay Ousterhout <[hidden email]>
> > wrote:
> > >> I agree with what Reynold said -- there's not a big benefit in terms
> of
> > >> lines of code (esp. compared to using getOrElse) and I think it hurts
> > code
> > >> readability.  One of the great things about the current Spark codebase
> > is
> > >> that it's very accessible for newcomers -- something that would be
> less
> > >> true with this use of "fold".
> > >>
> > >>
> > >> On Thu, Dec 26, 2013 at 8:11 PM, Holden Karau <[hidden email]>
> > wrote:
> > >>
> > >>> I personally with Evan in that I prefer map with getOrElse over fold
> > with
> > >>> options (but that just my personal preference) :)
> > >>>
> > >>>
> > >>> On Thu, Dec 26, 2013 at 7:58 PM, Reynold Xin <[hidden email]>
> > wrote:
> > >>>
> > >>>> I'm not strongly against Option.fold, but I find the readability
> > getting
> > >>>> worse for the use case you brought up.  For the use case of
> if/else, I
> > >>> find
> > >>>> Option.fold pretty confusing because it reverses the order of Some
> vs
> > >>> None.
> > >>>> Also, when code gets long, the lack of an obvious boundary (the only
> > >>>> boundary is "} {") with two closures is pretty confusing.
> > >>>>
> > >>>>
> > >>>> On Thu, Dec 26, 2013 at 4:23 PM, Mark Hamstra <
> > [hidden email]
> > >>>>> wrote:
> > >>>>
> > >>>>> On the contrary, it is the completely natural place for the initial
> > >>> value
> > >>>>> of the accumulator, and provides the expected result of folding
> over
> > an
> > >>>>> empty collection.
> > >>>>>
> > >>>>> scala> val l: List[Int] = List()
> > >>>>>
> > >>>>> l: List[Int] = List()
> > >>>>>
> > >>>>>
> > >>>>> scala> l.fold(42)(_ + _)
> > >>>>>
> > >>>>> res0: Int = 42
> > >>>>>
> > >>>>>
> > >>>>> scala> val o: Option[Int] = None
> > >>>>>
> > >>>>> o: Option[Int] = None
> > >>>>>
> > >>>>>
> > >>>>> scala> o.fold(42)(_ + 1)
> > >>>>>
> > >>>>> res1: Int = 42
> > >>>>>
> > >>>>>
> > >>>>> On Thu, Dec 26, 2013 at 5:51 PM, Evan Chan <[hidden email]> wrote:
> > >>>>>
> > >>>>>> +1 for using more functional idioms in general.
> > >>>>>>
> > >>>>>> That's a pretty clever use of `fold`, but putting the default
> > >>> condition
> > >>>>>> first there makes it not as intuitive.   What about the following,
> > >>>> which
> > >>>>>> are more readable?
> > >>>>>>
> > >>>>>>    option.map { a => someFuncMakesB() }
> > >>>>>>              .getOrElse(b)
> > >>>>>>
> > >>>>>>    option.map { a => someFuncMakesB() }
> > >>>>>>              .orElse { a => otherDefaultB() }.get
> > >>>>>>
> > >>>>>>
> > >>>>>> On Thu, Dec 26, 2013 at 12:33 PM, Mark Hamstra <
> > >>>> [hidden email]
> > >>>>>>> wrote:
> > >>>>>>
> > >>>>>>> In code added to Spark over the past several months, I'm glad to
> > >>> see
> > >>>>> more
> > >>>>>>> use of `foreach`, `for`, `map` and `flatMap` over `Option`
> instead
> > >>> of
> > >>>>>>> pattern matching boilerplate.  There are opportunities to push
> > >>>> `Option`
> > >>>>>>> idioms even further now that we are using Scala 2.10 in master,
> > >>> but I
> > >>>>>> want
> > >>>>>>> to discuss the issue here a little bit before committing code
> whose
> > >>>>> form
> > >>>>>>> may be a little unfamiliar to some Spark developers.
> > >>>>>>>
> > >>>>>>> In particular, I really like the use of `fold` with `Option` to
> > >>>> cleanly
> > >>>>>> an
> > >>>>>>> concisely express the "do something if the Option is None; do
> > >>>> something
> > >>>>>>> else with the thing contained in the Option if it is Some" code
> > >>>>> fragment.
> > >>>>>>>
> > >>>>>>> An example:
> > >>>>>>>
> > >>>>>>> Instead of...
> > >>>>>>>
> > >>>>>>> val driver = drivers.find(_.id == driverId)
> > >>>>>>> driver match {
> > >>>>>>>  case Some(d) =>
> > >>>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> > >>>>>>>    else {
> > >>>>>>>      d.worker.foreach { w =>
> > >>>>>>>        w.actor ! KillDriver(driverId)
> > >>>>>>>      }
> > >>>>>>>    }
> > >>>>>>>    val msg = s"Kill request for $driverId submitted"
> > >>>>>>>    logInfo(msg)
> > >>>>>>>    sender ! KillDriverResponse(true, msg)
> > >>>>>>>  case None =>
> > >>>>>>>    val msg = s"Could not find running driver $driverId"
> > >>>>>>>    logWarning(msg)
> > >>>>>>>    sender ! KillDriverResponse(false, msg)
> > >>>>>>> }
> > >>>>>>>
> > >>>>>>> ...using fold we end up with...
> > >>>>>>>
> > >>>>>>> driver.fold
> > >>>>>>>  {
> > >>>>>>>    val msg = s"Could not find running driver $driverId"
> > >>>>>>>    logWarning(msg)
> > >>>>>>>    sender ! KillDriverResponse(false, msg)
> > >>>>>>>  }
> > >>>>>>>  { d =>
> > >>>>>>>    if (waitingDrivers.contains(d)) { waitingDrivers -= d }
> > >>>>>>>    else {
> > >>>>>>>      d.worker.foreach { w =>
> > >>>>>>>        w.actor ! KillDriver(driverId)
> > >>>>>>>      }
> > >>>>>>>    }
> > >>>>>>>    val msg = s"Kill request for $driverId submitted"
> > >>>>>>>    logInfo(msg)
> > >>>>>>>    sender ! KillDriverResponse(true, msg)
> > >>>>>>>  }
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> So the basic pattern (and my proposed formatting standard) for
> > >>>> folding
> > >>>>>> over
> > >>>>>>> an `Option[A]` from which you need to produce a B (which may be
> > >>> Unit
> > >>>> if
> > >>>>>>> you're only interested in side effects) is:
> > >>>>>>>
> > >>>>>>> anOption.fold
> > >>>>>>>  {
> > >>>>>>>    // something that evaluates to a B if anOption = None
> > >>>>>>>  }
> > >>>>>>>  { a =>
> > >>>>>>>    // something that transforms `a` into a B if anOption =
> Some(a)
> > >>>>>>>  }
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> Any thoughts?  Does anyone really, really hate this style of
> coding
> > >>>> and
> > >>>>>>> oppose its use in Spark?
> > >>>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> --
> > >>>>>> --
> > >>>>>> Evan Chan
> > >>>>>> Staff Engineer
> > >>>>>> [hidden email]  |
> > >>>>>>
> > >>>>>> <http://www.ooyala.com/>
> > >>>>>> <http://www.facebook.com/ooyala><
> > >>>> http://www.linkedin.com/company/ooyala
> > >>>>>> <
> > >>>>>> http://www.twitter.com/ooyala>
> > >>>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>> --
> > >>> Cell : 425-233-8271
> > >>>
> >
>
Loading...