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 <[email protected]>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 <[email protected]> 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 <[email protected]
> > >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
> > [email protected] |
> >
> > <http://www.ooyala.com/>
> > <http://www.facebook.com/ooyala><http://www.linkedin.com/company/ooyala
> ><
> > http://www.twitter.com/ooyala>
> >
>
