Functional Aspects: Why Chaining Is Good

This is a short note about one of those things that annoy me quite often during code reviews. I am talking about primitive obsession used instead of some nicer alternatives. So let’s look at the code.

Here’s a simplified model class that stores amount of earned money and number of worked hours for a given day in an employee’s timesheet:

export class TimesheetDay {
  workDate: string;
  amount: number;
  hours: number;
  // ...
}

If we need to calculate the total amount of money earned by an employee within a given timeframe, the naive implementation would look like this:

public sumTimeSheetAmounts(weekDays: TimesheetDay[]): number {
  var sum = 0;
  for (let dayIndex = 0; dayIndex < days.length; dayIndex++) {
    const day = days[dayIndex];
    sum += day.amount;
  }
  return sum;
}

I see this kind of imperative bit-by-bit code in most of the pull requests. Does this code works? Yes, now it does. Is it easy to understand, maintain, extent? Definitely No, however even today in year 2017 many programmers would still disagree!

What’s wrong?

So why is this code poor and how can we improve it?

One important thing is that this code contains too many unnecessary details, especially if we think how simple is the problem being solved. Why does the author think that the ‘for’ loop is needed, when in fact it pushes us towards the infamous off-by-one error?

Similarly, why does ‘sum’ variable needs to be declared, and what makes sure it’s manipulated in a right way? Same question applies to the ‘day’ constant used in loop body.

All these things are just noisy details that distract the reader from the most important thing he or she is focused at: understanding what the code does, not how.

Refactoring: No Explicit For-Loop

Okay, now when we got very skeptical about the code snippet, let’s clean it. First, remove all the loop-related hustle. A simple forEach may come handy:

public sumTimeSheetAmounts(weekDays: TimesheetDay[]): number {
  var sum = 0;
  days.forEach(day => sum += day.amount);
  return sum;
}

We replaced 6 heavy lines of code with 3 lines. See how the intent becomes clearer. But we can do even better, since ugly ‘sum’ is still out there.

Refactoring: No Explicit Variable Declarations

public sumTimeSheetAmounts(days: TimesheetDay[]): number {
  return days
    .map(day => day.amount)
    .reduce((subTotal, current) => subTotal + current, 0);
}

Hmmm. This refactoring step is more controversial. Let me explain why I believe this code is better than the previous.

Firstly, we achieved the code that does not declare a single explicit variable. All we have is the ‘days’, ‘day’, ‘subtotal’, and ‘current’ symbols, each of them refers to something that is being provided from the outside rather than introduced by our code. There is an important byproduct: each of this “variables” has the shortest scope (or lifetime) possible. This is a huge achievement, because now the code makes it harder to make a wrong assignment happen. As a rule of thumb, we want as few state to deal with since it’s really easy to mess it up.

Secondly, we are now looking at more declarative code that tells us only the important steps and how they relate to each other. Namely, step A: take all the days one by one and extract their amount field values. Step B: sum up all the amount values, starting with the default value of an accumulator equal to zero.

Some may argue that we didn’t reduce the number of lines of code, and I will agree. However, we haven’t worsened this metric too. Another argument could point to the harder to understand concepts of map/reduce. While this may seem to be true, many developers are in agreement that map/reduce/filter/… are the functions that only confuse for a short time. These functions become programmer’s second nature when there’s a habit developed.

In other words, we indeed traded off a bit of simplicity for more defensiveness. It is harder to break the code now.

Ok, But Is It Future Friendly?

A valid question to ask now is what happens if we get an extra requirement that demands us to do something similar? Say, users want to calculate the total hours reported for a given time frame. Well, now we can write a super similar code to do that (as we know the code is sweet and nice).

public sumTimeSheetHours(days: TimesheetDay[]): number {
  return days
    .map(day => day.hours)
    .reduce((subTotal, current) => subTotal + current, 0);
}

Yes, this is it! We only need to copy-paste the code and change the field name we want to get summed up. It works neatly and is written in a similar (i.e. consistent) way!

Wait A Minute! Copy-Paste?

Yup. Copy paste is bad. Now we have two chunks of code that do similar things the same way (with the only small difference of the target of the operation — amount vs hours). Intuitively, we feel that if one function will need to change the other will have to change too.

To apply the DRY principle, all we need to do is basically extract the common parts into a separate function and reuse it in two places:

public sumTimeSheetAmounts(days: TimesheetDay[]): number {
  return this.sum(days, item => item.amount);
}

public sumTimeSheetHours(days: TimesheetDay[]): number {
  return this.sum(days, item => item.hours);
}

private sum(days: TimesheetDay[], fieldSelector: (_: TimesheetDay) => number): number {
  return days
    .map(fieldSelector)
    .reduce((subTotal, current) => subTotal + current, 0);
}

Now there’s no repeated implementation code at all; and if we wanted to add a new common rule there would be only one place to change. For example, we could prevent all the weekend days affecting the calculated sums:

private sum(days: TimesheetDay[], fieldSelector: (_: TimesheetDay) => number): number {
  return days
    .filter(day => dateFns.isWeekend(day.workDate) === false)
    .map(fieldSelector)
    .reduce((subTotal, current) => subTotal + current, 0);
}

Resume

Direct state manipulating code is evil. To avoid it, it’s very often possible and not hard to apply pure function chaining which is just one of the many efficient techniques in functional programming.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s