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 <c...@adatao.com> 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 > <nick.pentre...@gmail.com>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 <matei.zaha...@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 <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 > > >>> > > >
