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 <hol...@pigscanfly.ca> 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 <r...@databricks.com> 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 <m...@clearstorydata.com
> > >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 <e...@ooyala.com> 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 <
> > m...@clearstorydata.com
> > > > >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
> > > > e...@ooyala.com |
> > > >
> > > > <http://www.ooyala.com/>
> > > > <http://www.facebook.com/ooyala><
> > http://www.linkedin.com/company/ooyala
> > > ><
> > > > http://www.twitter.com/ooyala>
> > > >
> > >
> >
>
>
>
> --
> Cell : 425-233-8271
>
