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 |
>>
>
>
