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 <k...@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 <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
>>
