spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mark Hamstra <m...@clearstorydata.com>
Subject Re: Question about Scala style, explicit typing within transformation functions and anonymous val.
Date Sun, 17 Apr 2016 07:02:07 GMT
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 <gurwls223@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 <mark@clearstorydata.com>:
>
>> FWIW, 3 should work as just `.map(function)`.
>>
>> On Sat, Apr 16, 2016 at 11:48 PM, Reynold Xin <rxin@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 <gurwls223@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!
>>>>
>>>
>>>
>>
>

Mime
View raw message