Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarifying whitespace guidelines for classes #81

Open
MrPowers opened this issue Jan 5, 2021 · 0 comments
Open

Clarifying whitespace guidelines for classes #81

MrPowers opened this issue Jan 5, 2021 · 0 comments

Comments

@MrPowers
Copy link
Contributor

MrPowers commented Jan 5, 2021

Scala class definitions are often over 100 chars, so it'll be useful to give some clear guidance. The purpose of this issue is to clarify the whitespace guidelines for the different scenarios. Once that's clarified, I'll update the style guide with a PR detailing the scenarios.

Thanks again for making this style guide. It's been incredibly helpful over the years.

Scenario 1: When possible, put everything on one line

class Bar(val param1: String) extends BarInterface

Scenario 2: (params + traits) > 100 chars

The style guide currently has this:

class Foo(
    val param1: String,  // 4 space indent for parameters
    val param2: String,
    val param3: Array[Byte])
  extends FooInterface  // 2 space indent here
  with Logging {

  def firstMethod(): Unit = { ... }  // blank line above
}

Here's the formatting I'm actually seeing in the Spark codebase:

class Foo(val param1: String, val param2: String, val param3: Array[Byte])
  extends FooInterface with Logging {

  def firstMethod(): Unit = { ... }  // blank line above
}

Here's some actual Spark code:

case class SubtractTimestamps(endTimestamp: Expression, startTimestamp: Expression)
  extends BinaryExpression with ExpectsInputTypes with NullIntolerant {

Seems like the style guide is suggesting we should break up SubtractTimestamps to 6 lines. I like the 2 line formatting personally.

Scenario 3: params > 100 chars and traits > 100 chars

Here's another actual Spark class:

case class MonthsBetween(
    date1: Expression,
    date2: Expression,
    roundOff: Expression,
    timeZoneId: Option[String] = None)
  extends TernaryExpression with TimeZoneAwareExpression with ImplicitCastInputTypes
    with NullIntolerant {

Is the rule "when the traits > 100 chars, then they should all be put on a newline with 2 space indentation"? e.g. is this the desired formatting?

case class MonthsBetween(
    date1: Expression,
    date2: Expression,
    roundOff: Expression,
    timeZoneId: Option[String] = None)
  extends TernaryExpression
  with TimeZoneAwareExpression
  with ImplicitCastInputTypes
  with NullIntolerant {

Or do we want this?

case class MonthsBetween(
    date1: Expression,
    date2: Expression,
    roundOff: Expression,
    timeZoneId: Option[String] = None)
  extends TernaryExpression with TimeZoneAwareExpression with ImplicitCastInputTypes
  with NullIntolerant {

Scenario 4: params < 100 chars and traits > 100 chars

Is this the correct formatting?

case class AddHours(startTime: Expression, numHours: Expression, timeZoneId: Option[String] = None)
  extends BinaryExpression with ImplicitCastInputTypes with NullIntolerant
  with TimeZoneAwareExpression {

Or is it like the params - whenever the traits > 100 chars, then each trait needs to be broken out on a newline.

Thank you for the help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant