Skip to Content
Course content

101: Code Review Checklist for R Scripts

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

I just opened a PR from one of our interns. It's a script meant to process some monthly sales data and calculate a weighted performance score. On the surface, it works—the numbers come out right. But if I merged this as-is, we'd be paying for it in technical debt and compute costs six months from now. Let's walk through this together as I review it, because this is exactly how you should be looking at any R script before it hits production.

Wait, why is this taking so long?

First, I'm looking at the core calculation. The intern wrote this:

# Original code in the PR
results <- c() 
for (i in 1:nrow(sales_data)) {
  score <- sales_data$revenue[i] * sales_data$multiplier[i]
  results <- c(results, score)
}
sales_data$final_score <- results

I ran this on a small test set of 100 rows, and it felt instant. But then I swapped in the actual production set of 500,000 rows. My laptop fan started screaming, and the script just hung. I know exactly what's happening here: vector growth. Every time c(results, score) is called, R has to find a new, larger chunk of memory, copy the entire existing vector into it, and then add the new value. It's an $O(n^2)$ nightmare.

If I see this in a review, it's an immediate red flag. I'll suggest pre-allocating the vector if a loop is absolutely necessary, but in R, we should be asking: "Why is there a loop here at all?"

Thinking in vectors, not rows

I'm going to try to rewrite this the "R way." Instead of iterating through rows, I'll just multiply the columns directly.

# My adjusted version
sales_data$final_score <- sales_data$revenue * sales_data$multiplier

I ran this on that same 500k row dataset. It finished in a fraction of a second. This is the first thing you should check in any R code review: Vectorization. If you see a for loop doing basic arithmetic or logical checks on a dataframe, it's almost always a sign that the author is thinking in C++ or Python rather than R. It's not just about speed; it's about readability. One line of vectorized code is much harder to mess up than five lines of loop logic.

The NA landmine

Now that it's fast, I'm checking the output. I noticed some of the final_score values are NA. I dig into the raw data and see that about 2% of the multiplier column is missing. The current script just lets those NAs propagate. Depending on the business logic, that might be fine, or it might be a disaster.

I'll try to see what happens if we want to treat those NAs as 1 (neutral) instead of letting them wipe out the whole score:

# Testing a fix for missing values
sales_data$multiplier_fixed <- ifelse(is.na(sales_data$multiplier), 1, sales_data$multiplier)
sales_data$final_score <- sales_data$revenue * sales_data$multiplier_fixed

When you're reviewing R code, look specifically for how NA is handled. Did the author use na.rm = TRUE in their sum() or mean() calls? If not, one single missing value in a million-row dataset will make the entire result NA. It's a classic R trap.

Cleaning up the mental clutter

Last thing. I noticed the script uses df and data as variable names. Now, I know it seems harmless, but data() is a built-in R function. When you name a dataframe data, you're masking that function. It doesn't always cause a crash, but it makes the code confusing for the next person.

I'm also seeing a mix of <- and = for assignment. While R allows both, the community standard is <- for assignment. Mixing them makes the code look like it was copy-pasted from three different StackOverflow threads. I'll leave a comment asking the author to standardize the assignment operators and rename data to something descriptive, like monthly_sales.

So, my mental checklist for this review became:

  • Memory: Are they growing vectors inside a loop?
  • Idioms: Can this loop be vectorized?
  • Integrity: How are NA values being handled?
  • Clarity: Are they masking base functions or using inconsistent style?



📋 Practical Task

Refactor the "Customer Lifetime Value" Script

You have been handed a script written by a colleague that calculates the Lifetime Value (LTV) of customers. The script is slow and prone to errors. Your task is to apply the code review principles learned in this lesson to fix it.

The problematic code:

# Load data
data <- read.csv("customer_data.csv") 

# Calculate LTV
ltv_results <- c()
for(i in 1:nrow(data)) {
  # Calculate LTV: avg_purchase * frequency * lifespan
  val <- data$avg_purchase[i] * data$frequency[i] * data$lifespan[i]
  ltv_results <- c(ltv_results, val)
}
data$ltv <- ltv_results

# Calculate the average LTV across all customers
avg_ltv <- mean(data$ltv)
print(avg_ltv)

Your Requirements:

  • Rename the data variable to avoid masking the base R function.
  • Replace the for loop and vector growth with a vectorized operation.
  • Ensure the final mean() calculation doesn't return NA if some customers have missing data.
  • Standardize all assignments to use the <- operator.
Rating
0 0

There are no comments for now.

to be the first to leave a comment.