spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hamstra <m...@clearstorydata.com>
Subject Re: Option folding idiom
Date Fri, 27 Dec 2013 08:29:48 GMT
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 <matei.zaharia@gmail.com>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 <keo@eecs.berkeley.edu>
> 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 <holden@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 <rxin@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 <mark@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 <ev@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 <
> >>> mark@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
> >>>>> ev@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
> >>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message