spark-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Armbrust <mich...@databricks.com>
Subject Re: Proposal: Clarifying minor points of Scala style
Date Mon, 10 Feb 2014 22:56:50 GMT
+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 <
shivaram@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 <ilikerps@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
> > <shivaram@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 <rxin@databricks.com>
> wrote:
> >> > +1 on both
> >> >
> >> >
> >> > On Mon, Feb 10, 2014 at 1:34 AM, Aaron Davidson <ilikerps@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?
> >> >>
> >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message