GitHub PullRequestReviewCommentEvent at 2014-05-11 11:15:29
Repository: spark
Actor: jaceklaskowski
Payload: {"comment":{"url":"https://api.github.com/repos/apache/spark/pulls/comments/12508408","id":12508408,"diff_hunk":"@@ -297,7 +273,7 @@ object SparkBuild extends Build {\n   val chillVersion = \"0.3.6\"\n   val codahaleMetricsVersion = \"3.0.0\"\n   val jblasVersion = \"1.2.3\"\n-  val jets3tVersion = if (\"^2\\\\.[3-9]+\".r.findFirstIn(hadoopVersion).isDefined) \"0.9.0\" else \"0.7.1\"\n+  val jets3tVersion = \"^2\\\\.[3-9]+\".r.findFirstIn(hadoopVersion).fold(\"0.7.1\")(_ => \"0.9.0\")","path":"project/SparkBuild.scala","position":265,"original_position":265,"commit_id":"cecf74017b87758f7ec2ac5de345227953008e55","original_commit_id":"cecf74017b87758f7ec2ac5de345227953008e55","user":{"login":"jaceklaskowski","id":62313,"avatar_url":"https://avatars.githubusercontent.com/u/62313?","gravatar_id":"85c0f5fdbb31b1931ecefa08402c2107","url":"https://api.github.com/users/jaceklaskowski","html_url":"https://github.com/jaceklaskowski","followers_url":"https://api.github.com/users/jaceklaskowski/followers","following_url":"https://api.github.com/users/jaceklaskowski/following{/other_user}","gists_url":"https://api.github.com/users/jaceklaskowski/gists{/gist_id}","starred_url":"https://api.github.com/users/jaceklaskowski/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jaceklaskowski/subscriptions","organizations_url":"https://api.github.com/users/jaceklaskowski/orgs","repos_url":"https://api.github.com/users/jaceklaskowski/repos","events_url":"https://api.github.com/users/jaceklaskowski/events{/privacy}","received_events_url":"https://api.github.com/users/jaceklaskowski/received_events","type":"User","site_admin":false},"body":"I myself learnt it not so long ago and noticed it spurred some discussions about it in the Scala community (with [Martin Ordersky himself](http://stackoverflow.com/a/5332657/1305344)).\r\n\r\n[Functional Programming in Scala](http://www.manning.com/bjarnason/) reads on page 69 (in the pdf version):\r\n\r\n*\"It’s fine to use pattern matching, though you should be able to implement all the functions besides map and getOrElse without resorting to pattern matching.\"*\r\n\r\nSo I followed the advice and applied Option.map(...).getOrElse(...) quite intensively, but...IntelliJ IDEA just before I committed the change suggested to replace it with fold. I thought I'd need to change my habits once more and sent the PR.\r\n\r\nWith all that said, it's not clear what's the most idiomatic approach, but [pattern matching is in my opinion a step back from map/getOrElse](https://github.com/scala/scala/blob/2.12.x/src/library/scala/Option.scala#L156) and there's no need for it.\r\n\r\nI'd appreciate being corrected and could even replace `Option.fold` with `map`/`getOrElse` if that would make the PR better for more committers.","created_at":"2014-05-11T11:15:29Z","updated_at":"2014-05-11T11:15:29Z","html_url":"https://github.com/apache/spark/pull/706#discussion_r12508408","pull_request_url":"https://api.github.com/repos/apache/spark/pulls/706","_links":{"self":{"href":"https://api.github.com/repos/apache/spark/pulls/comments/12508408"},"html":{"href":"https://github.com/apache/spark/pull/706#discussion_r12508408"},"pull_request":{"href":"https://api.github.com/repos/apache/spark/pulls/706"}}}}