-
Notifications
You must be signed in to change notification settings - Fork 3
Phoenix Scala style guide
We have a lot of occurrences of "Root" pattern, where response object is defined like
object FooResponse {
case class Root(a: A, b: B)
def build(c: C, d: D): Root
}
This pattern has no pros, but has a very annoying cons: when multiple responses are used in the same file, imports have to be aliased:
import responses.FooResponse.{Root ⇒ FooRoot}
import responses.BarResponse.{Root ⇒ BarRoot}
which creates a lot of mess.
Please favor case class + companion object:
case class FooResponse(a: A, b: B)
object FooResponse {
def build(c: C, d: D): FooResponse
}
Requires good reason to be implemented. For any important data, avoid. Examples to avoid:
case class Foo(a: A, b: B, isActive = false)
case class CartTotals(subTotal: Int = 0, shippingTotal: Int = 0, taxes: Int = 0)
In these examples, it's very easy to forget to override one of default values. This leads to bugs. Prefer to provide values explicitly. Acceptable example:
case class Order(refNum: String, state: Order.State, remorsePeriodEnd: Option[Instant] = None)
Here, remorsePeriodEnd
only exists when state = RemorsePeriod
, so it's more or less acceptable. Ideally, this business logic should be supported by appropriate data structures, but this example is less harmful than the ones above because state transition is processes explicitly and can be supported via validation.
Discouraged. Provide methods with different signatures for different cases. Example:
Definition site:
def bad(foo: Option[A] = None, bar: Option[B] = None) = ...
Call site:
bad(foo = Some(a))
bad(bar = Some(b))
Replace with either separate methods with different signatures and required arguments
def goodA(x: A)
def goodB(x: B)
or just force call site to specify values explicitly, i.e. get rid of default values
def good(foo: Option[A], bar: Option[B])
Both are acceptable depending on your case.
- For public: always. TODO: enforce via scalastyle
- For private: preferably. Simplifies reading code
Covered by scalafmt