It's actually a little more complicated than that, mostly due to the
difference between private and private[this]. Allow me to demonstrate:
package dummy
class Foo1(a: Int, b: Int) {
private val c = a + b
}
class Foo2(a: Int, b: Int) {
private[this] val c = a + b
}
class Foo3(a: Int, b: Int) {
def getB = b
}
class Foo4(a: Int, private val b: Int) {
def getB = b
}
class Foo5(a: Int, private[this] val b: Int) {
def getB = b
}
scalac Foo.scala
Now let's look at what we've got using `javap -c -private` on the resulting
classes:
public class dummy.Foo1 {
private final int c;
private int c();
Code:
0: aload_0
1: getfield #13 // Field c:I
4: ireturn
public dummy.Foo1(int, int);
Code:
0: aload_0
1: invokespecial #20 // Method
java/lang/Object."<init>":()V
4: aload_0
5: iload_1
6: iload_2
7: iadd
8: putfield #13 // Field c:I
11: return
}
...compare that to Foo2 and note how `c` isn't a plain field in Foo1, but
has an accessor method `private int c()`:
public class dummy.Foo2 {
private final int c;
public dummy.Foo2(int, int);
Code:
0: aload_0
1: invokespecial #15 // Method
java/lang/Object."<init>":()V
4: aload_0
5: iload_1
6: iload_2
7: iadd
8: putfield #17 // Field c:I
11: return
}
Okay? So you really need private[this] if you want to generate plain Java
fields.
Now let's look at what happens when you close over an unannotated
constructor parameter:
public class dummy.Foo3 {
private final int b;
public int getB();
Code:
0: aload_0
1: getfield #14 // Field b:I
4: ireturn
public dummy.Foo3(int, int);
Code:
0: aload_0
1: iload_2
2: putfield #14 // Field b:I
5: aload_0
6: invokespecial #21 // Method
java/lang/Object."<init>":()V
9: return
}
...which is not the same as what you get if you annotate `b` with `private
val` -- notice `private int b()`:
public class dummy.Foo4 {
private final int b;
private int b();
Code:
0: aload_0
1: getfield #13 // Field b:I
4: ireturn
public int getB();
Code:
0: aload_0
1: invokespecial #18 // Method b:()I
4: ireturn
public dummy.Foo4(int, int);
Code:
0: aload_0
1: iload_2
2: putfield #13 // Field b:I
5: aload_0
6: invokespecial #23 // Method
java/lang/Object."<init>":()V
9: return
}
...however, compare Foo3 to what you get when `b` is `private[this] val`:
public class dummy.Foo5 {
private final int b;
public int getB();
Code:
0: aload_0
1: getfield #14 // Field b:I
4: ireturn
public dummy.Foo5(int, int);
Code:
0: aload_0
1: iload_2
2: putfield #14 // Field b:I
5: aload_0
6: invokespecial #21 // Method
java/lang/Object."<init>":()V
9: return
}
On Wed, Feb 12, 2014 at 1:29 PM, Aaron Davidson <ilike...@gmail.com> wrote:
> Regarding styling: as we all know, constructor parameters in Scala are
> automatically upgraded to "private val" fields if they're referenced
> outside of the constructor. For instance:
> class Foo(a: Int, b: Int) {
> def getB = b
> }
>
> In the above case, 'b' is actually a "private val" field of Foo, whereas
> 'a' never left the constructor's stack.
>
> Is this usage kosher, or should we prefer that such fields are marked
> "private val" explicitly if they're intended to be used outside of the
> constructor? This behavior is often harmless, but it has some evil
> implications with regards to serialization and shadowing during
> inheritance. It's especially concerning when a field starts out as a
> constructor parameter and during a later patch becomes a field, and now
> we're serializing it.
>
>
> On Mon, Feb 10, 2014 at 9:00 PM, Aaron Davidson <ilike...@gmail.com>
> wrote:
>
> > Alright, makes sense -- consistency is more important than special casing
> > for possible readability benefits. That is one of the main points behind
> a
> > style guide after all. I switch my vote for (1) to Shivaram's proposal as
> > well.
> >
> >
> > On Mon, Feb 10, 2014 at 4:40 PM, Evan Chan <e...@ooyala.com> wrote:
> >
> >> +1 to the proposal.
> >>
> >> On Mon, Feb 10, 2014 at 2:56 PM, Michael Armbrust
> >> <mich...@databricks.com> wrote:
> >> > +1 to Shivaram's proposal. I think we should try to avoid functions
> >> with
> >> > many args as much as possible so having a high vertical cost here
> isn't
> >> the
> >> > worst thing. I also like the visual consistency.
> >> >
> >> > FWIW, (based on a cursory inspection) in the scala compiler they don't
> >> seem
> >> > to ever orphan the return type from the closing parenthese. It seems
> >> there
> >> > are two main accepted styles:
> >> >
> >> > def mkSlowPathDef(clazz: Symbol, lzyVal: Symbol, cond: Tree,
> >> syncBody:
> >> > List[Tree],
> >> > stats: List[Tree], retVal: Tree): Tree = {
> >> >
> >> > and
> >> >
> >> > def tryToSetIfExists(
> >> > cmd: String,
> >> > args: List[String],
> >> > setter: (Setting) => (List[String] => Option[List[String]])
> >> > ): Option[List[String]] =
> >> >
> >> >
> >> > On Mon, Feb 10, 2014 at 2:36 PM, Shivaram Venkataraman <
> >> > shiva...@eecs.berkeley.edu> wrote:
> >> >
> >> >> Yeah that was my proposal - Essentially we can just have two styles:
> >> >> The entire function + parameterList + return type fits in one line or
> >> >> when it doesn't we wrap parameters into lines.
> >> >> I agree that it makes the code a more verbose, but it'll make code
> >> >> style more consistent.
> >> >>
> >> >> Shivaram
> >> >>
> >> >> On Mon, Feb 10, 2014 at 2:13 PM, Aaron Davidson <ilike...@gmail.com>
> >> >> wrote:
> >> >> > Shivaram, is your recommendation to wrap the parameter list even if
> >> it
> >> >> fits,
> >> >> > but just the return type doesn't? Personally, I think the cost of
> >> moving
> >> >> > from a single-line parameter list to an n-ine list is pretty high,
> >> as it
> >> >> > takes up a lot more space. I am even in favor of allowing a
> parameter
> >> >> list
> >> >> > to overflow into a second line (but not a third) instead of
> spreading
> >> >> them
> >> >> > out, if it's a private helper method (where the parameters are
> >> probably
> >> >> not
> >> >> > as important as the implementation, unlike a public API).
> >> >> >
> >> >> >
> >> >> > On Mon, Feb 10, 2014 at 1:42 PM, Shivaram Venkataraman
> >> >> > <shiva...@eecs.berkeley.edu> wrote:
> >> >> >>
> >> >> >> For the 1st case wouldn't it be better to just wrap the parameters
> >> to
> >> >> >> the next line as we do in other cases ? For example
> >> >> >>
> >> >> >> def longMethodName(
> >> >> >> param1,
> >> >> >> param2, ...) : Long = {
> >> >> >> }
> >> >> >>
> >> >> >> Are there a lot functions which use the old format ? Can we just
> >> stick
> >> >> >> to the above for new functions ?
> >> >> >>
> >> >> >> Thanks
> >> >> >> Shivaram
> >> >> >>
> >> >> >> On Mon, Feb 10, 2014 at 11:33 AM, Reynold Xin <
> r...@databricks.com>
> >> >> wrote:
> >> >> >> > +1 on both
> >> >> >> >
> >> >> >> >
> >> >> >> > On Mon, Feb 10, 2014 at 1:34 AM, Aaron Davidson <
> >> ilike...@gmail.com>
> >> >> >> > wrote:
> >> >> >> >
> >> >> >> >> There are a few bits of the Scala style that are underspecified
> >> by
> >> >> >> >> both the Scala
> >> >> >> >> style guide <http://docs.scala-lang.org/style/> and our own
> >> >> >> >> supplemental
> >> >> >> >> notes<
> >> >> >> >>
> >> >> >> >>
> >> >>
> >>
> https://cwiki.apache.org/confluence/display/SPARK/Spark+Code+Style+Guide
> >> >.
> >> >> >> >> Often, this leads to inconsistent formatting within the
> >> codebase, so
> >> >> >> >> I'd
> >> >> >> >> like to propose some general guidelines which we can add to the
> >> wiki
> >> >> >> >> and
> >> >> >> >> use in the future:
> >> >> >> >>
> >> >> >> >> 1) Line-wrapped method return type is indented with two spaces:
> >> >> >> >> def longMethodName(... long param list ...)
> >> >> >> >> : Long = {
> >> >> >> >> 2
> >> >> >> >> }
> >> >> >> >>
> >> >> >> >> *Justification: *I think this is the most commonly used style
> in
> >> >> Spark
> >> >> >> >> today. It's also similar to the "extends" style used in
> classes,
> >> with
> >> >> >> >> the
> >> >> >> >> same justification: it is visually distinguished from the
> >> 4-indented
> >> >> >> >> parameter list.
> >> >> >> >>
> >> >> >> >> 2) URLs and code examples in comments should not be
> line-wrapped.
> >> >> >> >> Here<
> >> >> >> >>
> >> >> >> >>
> >> >>
> >>
> https://github.com/apache/incubator-spark/pull/557/files#diff-c338f10f3567d4c1d7fec4bf9e2677e1L29
> >> >> >> >> >is
> >> >> >> >> an example of the latter.
> >> >> >> >>
> >> >> >> >> *Justification*: Line-wrapping can cause confusion when trying
> to
> >> >> >> >> copy-paste a URL or command. Can additionally cause IDE issues
> >> or,
> >> >> >> >> avoidably, Javadoc issues.
> >> >> >> >>
> >> >> >> >> Any thoughts on these, or additional style issues not
> explicitly
> >> >> >> >> covered in
> >> >> >> >> either the Scala style guide or Spark wiki?
> >> >> >> >>
> >> >> >
> >> >> >
> >> >>
> >>
> >>
> >>
> >> --
> >> --
> >> Evan Chan
> >> Staff Engineer
> >> e...@ooyala.com |
> >>
> >
> >
>
