Scala Style Guide

References

https://github.com/alexandru/scala-best-practices

Cosmetics

Follow scala-lang style guide with the following additions.

// For many attributes
case class User(
  many: Any,
  values: Any,
  like: Any,
  this: Any
)

// For long method params
def myMethod(
  many: Any,
  params: Any,
  like: Any,
  this: Any
): Any = {
 many.toString + params...
}

// For long chaining (a made-up example you should avoid, but if you must)
val foo = myListOfIds.
    filter(_ > 1000L).
    map(_ -> 1).
    reduceByKey(_ + _).
    map { case (id, count) => id * count }.
    filter(_ < 2000L).
    map { itemValue =>
      itemValue + mySupercaliFragilisticExpialidociousVarA +
        mySupercaliFragilisticExpialidociousVarB
    }.
    reduce(_ + _)

Style for Common Practices

Constructor-injection

User.scala

case class User(
  id: String,
  name: String,
  age: Int
)

UserRepository.scala

trait UserRepository {
  def fetchAll: List[User]
  def fetch(id: String): User
}

HttpUserRepository.scala

class HttpUserRepository(configUrl: String) extends UserRepository {
  def fetchAll = ...
  def fetch(id: String) = ...
}

object DefaultHttpUserRepository extends HttpUserRepository("http://localhost:9000/")

UsersController.scala

class UsersController(userRepository: UserRepository) { 
  def index = userRepository.fetchAll
}

object DefaultUsersController extends UsersController(DefaultHttpUserRepository)

Cake pattern

User.scala

case class User(id: String)

UserRepository.scala

trait UserRepositoryComponent {
  def userRepository: UserRepository
}

trait UserRepository {
  def fetchAll: List[User]
  def fetch(id: String): User
}

HttpUserRepository.scala

trait HttpUserRepository extends UserRepository {
  def configUrl: String

  def fetchAll = ...
  def fetch(id: String) = ...
}

object DefaultHttpUserRepository extends HttpUserRepository {
  val configUrl = "http://localhost:9000/"
}

UsersController.scala

trait UsersController { dependsOn: UserRepositoryComponent =>
  def index = userRepository.fetchAll
}

object DefaultUsersController extends UsersController with UserRepositoryComponent {
  val userRepository = DefaultHttpUserRepository
}

Implicit classes for type conversion

Classes for type conversion are a great usage of scala's implicits. Use them where applicable for cleaner code.

// Preferred
implicit class PimpedOutList[T](t: List[T]) {
  def asOther: MyOtherType[T] = {}
}

// Discouraged
def convertToOther[T](l: List[T]): MyOtherType[T] = ...

// Usage
val l: List[String] = List.empty
val o: MyOtherType[String] = l.asOther
// vs
val l: List[String] = List.empty
val o: MyOtherType[String] = convertToOther(l)

For-comprehensions

Use for-comprehensions when applicable for Future, Try, and Option. If your Futures don't depend on each other though, consider using Future.sequence instead as a for-comprehension will not launch them in parallel.

// Preferred
val finalResult: Future[Int] = for {
  firstResult <- myFutureApiCall
  secondResult <- myOtherFutureApiCall(firstResult)
  thirdResult <- yetAnotherFutureApiCall(secondResult)
} yield thirdResult

// Discouraged
val finalResult: Future[Int] = myFutureApiCall.flatMap { first =>
  myOtherFutureApiCall(firstResult).flatMap { second =>
    yetAnotherFutureApiCall(secondResult)
  }
}

// Discouraged (use Future.sequence instead)
val finalResult: Future[Int] = for {
  firstResult <- myFutureApiCall
  secondResult <- myOtherFutureApiCall
  thirdResult <- yetAnotherFutureApiCall
} yield firstResult + secondResult + thirdResult

Spaces vs. Periods

Always prefer periods over spaces unless doing so leads to ambiguity (in the multi-line case for map/filter/fold).

// Preferred 
myString.toLowerCase.split(",")

// Discouraged
myString toLowerCase split ","

// Preferred
myList.filter(_.isDefined).map(_.id).headOption.getOrElse(123)

// Discouraged 
myList filter _.isDefined map _.id headOption getOrElse 123

// Preferred
myList.filter { elem =>
  ...
} headOption

// Bad, looks like headOption is called on the block rather than the result
myList.filter { elem =>
  ...
}.headOption

Patterns

Use immutability

Immutability simplifies reasoning about your program and makes it much easier to use concurrency.

// Good
val l = List(1, 2, 3)
val s = l.sum

// Bad, don't use `var`
val l = List(1, 2, 3)
var s = 0
for(x <- l) s += x

Use pattern matching

Pattern matching is a powerful and clear way to convey what cases you care about, use it!

// Good
myList match {
  case head::Nil => head + 1
  case head::second::Nil => head + second * 3
  case _ => 0
}

// Bad
if (myList.length == 1) {
  myList(0) + 1
} else if (myList.length == 2) {
  myList(0) + myList(1) * 3
} else {
  0
}

Use `def` for all abstract values

Whenever declaring a member abstract, even if it is a value, use def instead. This gives the freedom to the implementer how to satisfy this requirement.

// Good
trait Foo {
  def configString: String
}

trait Foobar extends Foo {
  lazy val configString = "yay"
}

// Bad
trait Foo {
  val configString: String
}

trait Foobar extends Foo {
  // won't work :(
  def configString = MyStore.load("configString")
  lazy val configString = MyStore.expensiveLookup("configString")
}

Use `Try[T]` instead of throwing exception

When a function can knowingly error, capture this fact in the signature by having it return a Try[T] rather than throwing an exception.

// Good
def attemptToParse(s: String): Try[MyType] = {
  if (s.isValid) {
    Success(MyType(s))
  } else {
    Failure(MyParseException())
  }
}

def attemptToParse(s: String): Try[MyType] = Try {
  ... something that could fail
} recoverWith {
  case e: NoSuchElementException => Failure(MyParseException())
}

// Bad
def attemptToParse(s: String): MyType = {
  MyType(s) // this could fail for some reason
}

Use Hungarian notation when appropriate

When dealing with container objects such as Option, Try, or Future, use Hungarian notation to designate them.

// Good
val myNumFuture = Future.successful(1)
val myNumOptTry = Try { Some(2) }
val myNumOpt = myNumOptTry.getOrElse(None)
val myNum = myNumOpt.getOrElse(0)

Use explicit types for public methods

Scala's type inference can be specific in subtle ways that can lead to puzzling errors later on. Use explicit type declarations for all public methods.

// Good
def parse(s: Int): Option[JsValue] = {
  if (s > 10) Some(JsNumber(s)) else None
}

// Bad, type is actually Option[JsNumber]
def parse(s: Int) = {
  if (s > 10) Some(JsNumber(s)) else None
}

Capture execution context dependencies in method signatures

Bubble up dependency on an execution context to the method signature to let the implementer decide which to use.

// Good
def myBlockingCall(implicit ctx: ExecutionContext) = Future {
  ...
}
myBlockingCall(scala.concurrent.ExecutionContext.global)

// Bad
import scala.concurrent.ExecutionContext.global
def myBlockingCall() = Future {
  ...
}
myBlockingCall()

Anti-patterns

There are exceptions to every rule, but avoid these anti-patterns whenever possible.

Never write a function whose logic is more than 30 lines

If you find yourself writing a function longer than that, break it up into components. Monolithic functions are difficult to read and understand. Writing one-off functions in scala is incredibly easy, use it!

def myLongFunction = {
  def myFirstComponent = ...
  def mySecondComponent = ...
  def myThirdComponent = ...
  
  val intermediateResult = myFirstComponent.map(mySecondComponent)
  myThirdComponent(intermediateResult)
}

Never use return

Scala already returns the last value of your block, using return can only lead to difficult to understand and unexpected behavior that will be difficult to debug. Not to mention, return is implemented by throwing an exception which is never a good idea for efficient control flow.

Never catch `Throwable`

Global catching like this is dangerous, OutOfMemoryExceptions and other serious errors should be bubbled all the way to the top. Use NonFatal(e) instead.

// Good 
myTry recover {
  case NonFatal(e) => ...
}

// Bad
myTry recover {
  case e => ...
}

Never use `null` or `Option`.get

Scala has excellent support for optional types which nearly eliminates the pesky NullPointerException of old. Using null reintroduces these nasty bugs and Option.get just trades a NoSuchElementException for a NullPointerException. Handle optional values apporpriately and don't use null.

Never define a case class inside another class

Defining objects inside other classes cause those objects to contain references to the scope of the class thanks to closures. Case classes are for immutable data representation and have no business holding on to references of parent classes.

// Good
case class User(id: Int)
class UserRepository {
  ...
}

// Bad
class UserRepository {
  case class User(id: Int)
  ...
}

// OK
object UserRepository {
  case class User(id: Int)
  
  class UserRepository {
    ...
  }
}

patrickhulce/ScalaStyle.md