i find version 3 without the _ also more readable On Sun, Apr 17, 2016 at 3:02 AM, Mark Hamstra <m...@clearstorydata.com> wrote:
> I actually find my version of 3 more readable than the one with the `_`, > which looks too much like a partially applied function. It's a minor > issue, though. > > On Sat, Apr 16, 2016 at 11:56 PM, Hyukjin Kwon <gurwls...@gmail.com> > wrote: > >> Hi Mark, >> >> I know but that could harm readability. AFAIK, for this reason, that is >> not (or rarely) used in Spark. >> >> 2016-04-17 15:54 GMT+09:00 Mark Hamstra <m...@clearstorydata.com>: >> >>> FWIW, 3 should work as just `.map(function)`. >>> >>> On Sat, Apr 16, 2016 at 11:48 PM, Reynold Xin <r...@databricks.com> >>> wrote: >>> >>>> Hi Hyukjin, >>>> >>>> Thanks for asking. >>>> >>>> For 1 the change is almost always better. >>>> >>>> For 2 it depends on the context. In general if the type is not obvious, >>>> it helps readability to explicitly declare them. >>>> >>>> For 3 again it depends on context. >>>> >>>> >>>> So while it is a good idea to change 1 to reflect a more consistent >>>> code base (and maybe we should codify it), it is almost always a bad idea >>>> to change 2 and 3 just for the sake of changing them. >>>> >>>> >>>> >>>> On Sat, Apr 16, 2016 at 11:06 PM, Hyukjin Kwon <gurwls...@gmail.com> >>>> wrote: >>>> >>>>> Hi all, >>>>> >>>>> First of all, I am sorry that this is relatively trivial and too minor >>>>> but I just want to be clear on this and careful for the more PRs in the >>>>> future. >>>>> >>>>> Recently, I have submitted a PR ( >>>>> https://github.com/apache/spark/pull/12413) about Scala style and >>>>> this was merged. In this PR, I changed >>>>> >>>>> 1. >>>>> >>>>> from >>>>> >>>>> .map(item => { >>>>> ... >>>>> }) >>>>> >>>>> to >>>>> >>>>> .map { item => >>>>> ... >>>>> } >>>>> >>>>> >>>>> >>>>> 2. >>>>> from >>>>> >>>>> words.foreachRDD { (rdd: RDD[String], time: Time) => ... >>>>> >>>>> to >>>>> >>>>> words.foreachRDD { (rdd, time) => ... >>>>> >>>>> >>>>> >>>>> 3. >>>>> >>>>> from >>>>> >>>>> .map { x => >>>>> function(x) >>>>> } >>>>> >>>>> to >>>>> >>>>> .map(function(_)) >>>>> >>>>> >>>>> My question is, I think it looks 2. and 3. are arguable (please see >>>>> the discussion in the PR). >>>>> I agree that I might not have to change those in the future but I just >>>>> wonder if I should revert 2. and 3.. >>>>> >>>>> FYI, >>>>> - The usage of 2. is pretty rare. >>>>> - 3. is pretty a lot. but the PR corrects ones like above only when >>>>> the val within closure looks obviously meaningless (such as x or a) and >>>>> with only single line. >>>>> >>>>> I would appreciate that if you add some comments and opinions on this. >>>>> >>>>> Thanks! >>>>> >>>> >>>> >>> >> >
