Skip to Content
Course content

116: Code Review Checklist for Idiomatic Scala

Click on the "Edit" button in the top corner of the screen to edit your slide content.

By the time you've reached this point in the course, you know the syntax. But there is a massive gap between code that compiles in Scala and code that is idiomatic Scala. When I'm reviewing PRs for my team, I'm not looking for bugs—the compiler and the tests usually find those. I'm looking for "Java-isms" or "Python-isms" that make the code harder to reason about.

Let's walk through a real scenario. Imagine we're building a small system to process customer orders. We need to filter out cancelled orders, calculate the total for high-value customers, and handle potentially missing shipping addresses.

The starting point: Java-style Scala

Here is how a developer who is new to the functional paradigm might write this. It works, but it's noisy and fragile.

case class Order(id: Int, amount: Double, status: String, address: String)

def processOrders(orders: List[Order]): Double = {
  var total = 0.0
  for (order <- orders) {
    if (order.status != "CANCELLED") {
      if (order.address != null) {
        if (order.amount > 100.0) {
          total += order.amount
        }
      }
    }
  }
  total
}

If this landed on my desk, my first checklist item would be: "Are we using mutable state where a transformation would work?" That var total is a red flag. It forces the reader to keep a mental ticker of the state changing over time. In Scala, we prefer describing what the data is, not how to loop through it.

Swapping loops for functional pipelines

I'll start by replacing that for loop with a chain of transformations. This is the heart of idiomatic Scala: treating collections as streams of data.

def processOrders(orders: List[Order]): Double = {
  orders
    .filter(_.status != "CANCELLED")
    .filter(_.amount > 100.0)
    .map(_.amount)
    .sum
}

This is much cleaner. However, I noticed something in the original code: the null check on address. In my haste to make the pipeline pretty, I just deleted it. That's a mistake. If address is null, the original code skipped the order. By removing the check entirely, I've changed the business logic.

But wait—checking for null in Scala is an anti-pattern. My second checklist item is: "Is there a null check that should be an Option?"

Handling missing data without the dread

Let's fix the data model first. The address should be an Option[String]. This forces anyone using the Order class to acknowledge that the address might be missing, moving the error from runtime to compile-time.

case class Order(id: Int, amount: Double, status: String, address: Option[String])

def processOrders(orders: List[Order]): Double = {
  orders
    .filter(_.status != "CANCELLED")
    .filter(_.address.isDefined) // Still a bit clunky
    .filter(_.amount > 100.0)
    .map(_.amount)
    .sum
}

I'm still not happy. Using .isDefined is essentially just a null check with a different name. The idiomatic way to handle this is to use collect. It allows us to filter and map in a single pass using pattern matching.

The final idiomatic polish

Now I'll apply my final checklist item: "Can this be simplified with pattern matching or partial functions?"

def processOrders(orders: List[Order]): Double = {
  orders.collect {
    case Order(_, amount, status, Some(_)) if status != "CANCELLED" && amount > 100.0 => 
      amount
  }.sum
}

Look at the difference. We've gone from a nested mess of if statements and mutable variables to a single declarative statement. We're saying: "Collect the amounts of orders that have an address, aren't cancelled, and are over 100 dollars, then sum them up."

When you're reviewing your own code, ask yourself: Am I telling the computer how to move bits around (loops, vars, null checks), or am I describing the relationship between my input and my output?




📋 Practical Task

Refactoring the Inventory Stock Validator

You've been handed the following code snippet for a warehouse management system. It's functional, but it's written in a non-idiomatic, imperative style. Your task is to refactor it into idiomatic Scala.

case class Item(sku: String, quantity: Int, category: String, warehouseLocation: String)

def getLowStockValue(items: List[Item], threshold: Int): Double = {
  var totalValue = 0.0
  for (item <- items) {
    if (item.warehouseLocation != null) {
      if (item.quantity < threshold) {
        // Assume a helper function: Item.getPrice(sku)
        totalValue += Item.getPrice(item.sku) * item.quantity
      }
    }
  }
  totalValue
}

Requirements for your refactor:

  • Change warehouseLocation to use Option[String].
  • Remove the var and the for loop.
  • Use a functional pipeline (e.g., filter, map, collect, or foldLeft).
  • Ensure the logic remains the same: only items with a valid location and quantity below the threshold are summed.
Rating
0 0

There are no comments for now.

to be the first to leave a comment.