spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Reynold Xin <r...@databricks.com>
Subject Re: Question about Scala style, explicit typing within transformation functions and anonymous val.
Date Sun, 17 Apr 2016 06:48:28 GMT
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