Forem: Andrew Durber

Array.reduce considered harmful?

Andrew Durber — Mon, 14 Jun 2021 14:53:41 +0000

These past few weeks I've seen some stirrings on Twitter about how people should avoid using Array.prototype.reduce mainly around the fact it's difficult to read. At first I thought it was rubbish and that it wasn't difficult at all. The more I thought about it I realised that I've been writing JavaScript for years. I've led teams and projects, I've also been the guy that people come to for help with their JS. I'm an experienced developer.

What I was missing from my initial dismissal of the tweets versus reduce was empathy. I'll come back to that.

When I first came across reduce it took a while for it to stick in my brain, I studied it and practiced it and eventually I had the muscle memory to bend code to my will using reduce.

...but that's the crux of the issue isn't it? It took time and focus to understand.

Writing software is challenging enough - especially in large codebases where dragons are lurking around every corner - without fighting to understand the basic language.

My brain on reduce (at first)

Here's my inner dialogue when I first started coming across reduce:

// Okay so we're going to do something here
// It takes data as an argument and returns something
function doSomething(data){
// Nice a one liner this should be simple

// So I'm guessing 'data' is an array, I know that reduce is for arrays. (TypeScript helps with this!)

// Wait what is acc? What is curr?
// Nice argument names doofus.
// acc + curr.val, okay so is it concatenating strings? 
// Oh theres a second argument to reduce 
// *reads MDN docs*
// Oh that's the initial value
// Sweet so it's just calculating a total
// So on first pass acc = 0
// Ahh and then for each element of data we add the elements `val` property
    return data.reduce((acc, curr) => acc + curr.val, 0)
}

That's for a simple reduce, this is the primary example that's given on when to use reduce. reduce is good for other stuff such as grouping data for a given key or combining map and filter in a single iteration:

const activeIds = items
.filter((item) => item.active === true)
.map((item) => item.id)


const activeIds = items.reduce((result, item) => {
  if(!item.active) return result;
  return [...result, item.id]
}, [])

The filter + map loops over items twice whereas reduce does it once. However, tell me which of the above snippets is the easiest to read? It's filter + map, yes if you're familiar with reduce and your muscle memory kicked in it's not so bad. When we're building software we want to be focused on the business logic and adding features, not wasting focus deciphering language features.

Empathy

There's the common quote that's used:

Code is read much more than it is written.

This is 100% true.

You write some code
It works
You ship it
Time passes
Requirements change
You or some other poor soul needs to update the code
They study it and all code around it
They decide if this is the place the change needs to be made
They make the change

Repeat as many times as requirements change or bugs are fixed. Most of the time spent in that process is reading and understanding.

Now imagine someone on your team less experienced at JavaScript or software development in general comes along and not only do they need to understand the requirements of the task, but also break down the language. They won't have the muscle memory that you do.

So tell me why is:

function calculateTotalValue(data){
  return data.reduce((result, item) => result + item.val, 0)
}

better than this:

function calculateTotalValue(data){
  let sum = 0;
  // This could also easily be a straight up for loop
  for(let item of data){
    sum += i.val;
  } 
  return sum;
}

Anyone can read the loop, you don't get points for being succinct. This isn't code golf.

Performance

I very briefly touched on performance when I mentioned iterating a collection just once vs filter + map.

What if I told you that a regular for loop or a for..of loop was faster than reduce anyway? (marginally for realistic examples)

Take a look at these snippets

function reduceData(data){
  return data.reduce((acc, curr) => acc + curr.val, 0)
}

function forOfData(data){
  let sum = 0;
  for(let i of data){
    sum += i.val;
  } 
  return sum;
}

function forLoopData(data){
  let sum = 0;
  for(let i = 0, len = data.length; i < len; i++){
    sum += data[i].val;
  }
  return sum;
}

Result: 1000 items

Result: 10,000 items

You can view the benchmark here

Yes, these are really contrived and for realistic sample sizes it's extremely marginal.

Conclusion

Writing code isn't about telling a computer what to do. It's about telling other people what you want to computer to do. You don't get points for succinct code unless you're doing code golf. You don't get points for smashing out a 1 liner that takes 5 minutes to understand.

You get points from writing readable code, your team will thank you.

I wrote this post as a reminder to myself. I'm guilty of doing some pretty arcane stuff with reduce when I should reach for a simpler solution. I'm trying to be better.

So, use filter+map+whatever or regular loops:

✅ More readable
✅ Faster (marginally)
✅ Anyone with a basic understanding of JavaScript understands what's going on

Building a cache decorator to keep your app fast 🏎

Andrew Durber — Tue, 14 Jul 2020 20:37:05 +0000

Splash Photo by Jouwen Wang on Unsplash

People in 2020 expect apps to be fast. Really fast. Slow pages negatively affect conversions. Speed minimises user frustration.

More money and happy customers? I'll take it.

I spend a lot of time thinking about performance and there are lots of things to consider when building a high-performance application, but the single most important concept is "don't do work if you do not need to." Your code will never be faster than no code. Your API calls will never be faster not calling the API in the first place.

Background

In an application I'm building, we fetch a ton of data. After watching my network tab in Chrome Dev Tools as I navigated and interacted with the app, there were dozens of requests. Most of which don't change very much. Navigating around the app can cause data to be fetched multiple times or if the user reloads the page. The web app is a SPA, so thankfully full page loads are rare.

When we're caching we have two possible methods:

In-memory (simplest)
Persistent (not hard, but more difficult than in-memory)

I separated all my API calls into a service layer within the application, I apply all transforms and request batching there. I started with the slowest requests and built a simple TTL cache.

Using the cache was simple. I check if the cache has a value for the given cache key, if so return it. If not fetch the data and add the data to the cache when we get it.

Here's a link to the TTL Cache implementation if you're interested: Gist: TTL Cache

type MyData = { id: string; name: string }

const dataCache = new TTLCache<MyData>({ ttl: 60 })

async function fetchMyData(userId: string): Promise<MyData> {
  const cacheKey = `mydata:${userId}`
  if (dataCache.has(cacheKey)) {
    return dataCache.get(cacheKey)
  }

  // do API call
  const result = await Api.get('/my-data', { params: { userId } })
  if (result.data) {
    dataCache.set(cacheKey, result.data)
  }

  return result.data
}

The problem

After using this pattern with dozens of API calls, it started getting cumbersome. Caching should be a side-effect, I want to focus solely on what the code is doing.

After staring at my screen for a little while. Tilting my head and squinting. I decided to try and create an abstraction for this pattern.

The solution - Decorators!

We'll be building an in-memory cache here, but at the bottom I'll leave an implementation that uses IndexedDB for persistent caching.

Note

Using decorators required me to use a class but that was a minor inconvenience.

One of the first steps I take when designing an API for an abstraction is to write some code on how I want the code to look.

I wanted to be able to see that some call was cached but I didn't want it to take more than 3 lines of code to do so.
I just wanted to specify a cache key.
All arguments to the call must be serialized. So a change in the arguments returns fresh data.

Here's the code I wrote for my perfect API.


class UserService{
  @cache('mydata')
  async fetchMyData(userId:string):Promise<MyData>{
    const result = await Api.get('/my-data', { params: { userId } })
    return result.data
  }
}

MAGNIFICO!

I knew I could write a decorator that did this. However, a problem arose immediately: I'd need to initialise the cache(s) outside of the decorator.

The simple solution was to just create an object with the caches:

const caches = {
 myData: new TTLCache<MyData>({ ttl: 60 })
}

Quick aside: The anatomy of a decorator

function cache(cache: keyof typeof caches) { // The decorator factory
    return function(target: any, propertyKey: string, descriptor: PropertyDescriptor) { // The decorator

    }
  }

target is the class that the decorated function is on.
propertyKey is the name of the decorated function.
descriptor is the meat and potatoes. It's the function definition.

Implementation

So as a first step, let's create a decorator that just calls the function.


const caches = {
    myDataCache: new TTLCache<MyData>({ttl: 60})
}
function cache(cache: keyof typeof caches) {
    const ttlCache = caches[cache] // Get the cache instance
    return function(_: any, __: string, descriptor: PropertyDescriptor) {
      let method = descriptor.value // grab the method
// We want to override the method so lets give the method a new value. 
      descriptor.value = function() {
          // just call the original function
        return method.apply(this, arguments)
      }
    }
  }

Like I said, this does nothing. We've overridden the method...with itself?

Serialize the arguments

As I mentioned previously, we need to cache calls with different arguments separately.

"All arguments to the call must be serialized. So a change in the arguments returns fresh data."

Let's create a function that takes any number of arguments and stringifys them all:

const serializeArgs = (...args: any[]) =>
  args
    .map((arg: any) => arg.toString())
    .join(':')

Let's update our decorator value to include the cache key.

 descriptor.value = function() {
    const cacheKey = serializeArgs(...arguments)
    // call the function
    return method.apply(this, arguments)
}

We call it within the descriptor.value function to get the arguments of the called function

This creates a nice cache key:

@cache('myData')
async fetchMyData(userId:string){}

// lets say it was called with 1234
service.fetchMyData(1234)
// cache key is: myData1234

// if we had additional arguments
async fetchMyData(userId:string, status:string){}

service.fetchMyData(1234, 'ACTIVE')
// cache key is: myData1234:ACTIVE

Check if the cache has the value

Nice and simple:

descriptor.value = function() {
    const cacheKey = serializeArgs(...arguments)
    // Check if we have a cached value. 
    // We do it here before the method is actually called
    // We're short circuiting
    if (ttlCache.has(cacheKey)) {
        return ttlCache.get(cacheKey)
    }
    // call the function
    return method.apply(this, arguments)
}

Running the method and getting the result

I thought this was going to be more challenging, but after thinking about it, we know that the method returns a promise. So let's call it.

descriptor.value = function() {
    const cacheKey = serializeArgs(...arguments)
    if (ttlCache.has(cacheKey)) {
    return ttlCache.get(cacheKey)
    }

// We don't need to catch, let the consumer of this method worry about that
    return method.apply(this, arguments).then((result: any) => {
        // If we have a result, cache it!
    if (result) {
        ttlCache.set(cacheKey, result)
    }
    return result
    })
}

That's it! That's the full implementation of the cache.

We check if there's a value in the cache. If so then exit early with the cached value
We call the method, resolve the promise, if there is a value add it to the cache. Return the result.

You don't need to even use a TTL cache, you could user localStorage or whatever you wish.

Full implementation

Here's the full implementation if you're interested.

const caches = {
  myDataCache: new TTLCache<MyData>({ ttl: 60 }),
}
function cache(cache: keyof typeof caches) {
  const ttlCache = caches[cache] // Get the cache instance
  return function(_: any, __: string, descriptor: PropertyDescriptor) {
    let method = descriptor.value // grab the function
    descriptor.value = function() {
      const cacheKey = serializeArgs(...arguments)
      if (ttlCache.has(cacheKey)) {
        return ttlCache.get(cacheKey)
      }

      return method.apply(this, arguments).then((result: any) => {
        // If we have a result, cache it!
        if (result) {
          ttlCache.set(cacheKey, result)
        }
        return result
      })
    }
  }
}

Taking it further

An in-memory cache might not cut it. If you have data that you want to cache through reloads, you can use IndexedDB.
Here's an example of using money-clip, a TTL IndexedDB wrapper.

IndexedDB has an asynchronous API so we need to wrap the method call in a promise.

import {get, set} from 'money-clip'

export function persistentCache(key: string, maxAge: MaxAge) {
  const cacheOptions: Options = {
    version: extractNumberFromString(environment.version) || 1,
    maxAge: hmsToMs(maxAge.hours || 0, maxAge.minutes || 0, maxAge.seconds || 0) || 60 * 1000,
  }

  return function(_: any, __: string, descriptor: PropertyDescriptor) {
    let method = descriptor.value

    descriptor.value = function() {
      const cacheKey = serializeArgs(key, ...arguments)
      var args = arguments
      return get(cacheKey, cacheOptions).then((data) => {
        if (data) {
          return data
        }

        return method.apply(this, args).then(
          (result: any) => {
            if (result) {
              set(cacheKey, result, cacheOptions)
            }
            return result
          },
          () => {
            return method.apply(this, args)
          }
        )
      })
    }
  }
}

There's also nothing stopping you from using localStorage or sessionStorage. Anything where you can get and set values will work perfectly.

In search of a faster uniqueBy

Andrew Durber — Wed, 10 Jun 2020 17:39:54 +0000

In a project I'm working on I'm trying to keep it as lean as possible, which means I haven't reached for libraries like Lodash. Instead, I've challenged myself to hand-roll everything I need.

I needed to get an array of items unique by a given key, just like the Lodash uniqBy. I had a quick Google around to see how other folks are approaching it.

I came across the following approach:

function uniqueBy(myArr, prop) {
  // Iterate over the array and filter out duplicates
  return myArr.filter((obj, pos, arr) => {
    // Map over the array and get the values from the key.
    return arr.map(mapObj => mapObj[prop]).indexOf(obj[prop]) === pos
  })
}

While this works, I was not too fond of mapping inside the filter. So I set up some tests around my function and started created benchmarks on jsPerf.

With an array of 10,000 items, this had a whopping 0.63 ops/sec. Zoinks.

Note

I just want to clarify, I'm in no way trying to criticise the author of the above function. Most people can use that and not notice any negative performance impact.

Iteration 1

So I thought, what if I moved the map outside of the filter?

function uniqueBy(myArr, prop) {
  // Get all values for the prop up front
  const vals = myArr.map(obj => obj[prop])
  return myArr.filter((obj, pos, arr) => {
    return vals.indexOf(obj[prop]) === pos
  })
}

Result: 3,067 ops/sec
Extracting the map outside of the filter had much better results, relatively speaking.

Iteration 2

Keeping the same vibe, I moved onto Array.prototype.findIndex

function uniqueBy(arr, prop) {
  return arr.filter((record, index, self) => {
    // Instead of getting the values, just get the index where the predicate is true.
    return index === self.findIndex(t => t[prop] === record[prop])
  })
}

Result: 6,962 ops/sec

But this is much of muchness; this will still make multiple passes over the array—time to whip out the old trusty loops without predicates.

Iteration 3

function uniqueBy(arr, prop) {
  const len = arr.length // get the length up front to ensure it's only accessed once

  const data = [] // This will be our return data

  const seen = [] // This is a collection of values we've already seen

  for (let i = 0; i < len; i++) {
    // Get the things I care about here to only access the properties once.
    const item = arr[i] // The current array item

    const val = item[prop] // The current items' value that we want to unique by

    // If there's no record of this in "seen", push it to seen and add it to our return array
    // What's with the tilde? Since indexOf returns a number between -1 and N, the tilde (~) is used to convert that value into a boolean. It's the bitwise NOT operator. Link at the bottom.
    if (!~seen.indexOf(val)) {
      // Mark this value as seen
      seen.push(val)
      // Add the value to the return array
      data.push(item)
    }
  }

  return data
}

Result: 15,196 ops/sec swoon

So we managed to get rid of the predicate callbacks, our tests still pass, and it's faster. Now we're getting somewhere.
It's somewhat less readable than previous iterations, but that's not my goal. We could stop here, but I think we can squeeze some more out of this.

Iteration 4

What if we use a Set? They're pretty nifty right:

function uniqueBy(arr, prop) {
  const len = arr.length
  const data = []
  const seen = new Set() // Create a Set
  for (let i = 0; i < len; i++) {
    const item = arr[i]
    const val = item[prop]
    if (!seen.has(val)) {
      // Check if the set has the value
      seen.add(val)
      data.push(arr[i])
    }
  }

  return data
}

Result: 11,133 ops/sec

Wait a minute! That's slower than the previous one. Wha-, ugh-, but it's nifty! Ah well, on we go.

Iteration 5

After perusing over some benchmarks on loops, I saw that a while loop vastly outperformed a for loop.

function uniqueBy(arr, prop) {
  const len = arr.length
  const record = []
  const seen = []
  let cursor = 0
  while (cursor < len) {
    const item = arr[cursor]
    const val = item[prop]
    if (!~seen.indexOf(val)) {
      seen.push(val)
      record.push(item)
    }
    cursor++
  }
  return record
}

Result:: 15,164 ops/sec

Boom! A while loop made this one is our fastest one yet, but even less readable.

Iteration 6

Hmm, from the loop benchmarks, decrementing is faster than incrementing, how does that look?

function uniqueBy(arr, prop) {
  let len = arr.length
  const record = []
  const seen = []
  while (len--) {
    const item = arr[len]
    const val = item[prop]
    if (!~seen.indexOf(val)) {
      seen.push(val)
      record.push(item)
    }
  }
  return record
}

Result: 15,535 ops/sec

CAVEAT: We've lost the original order of the array.

These are marginal gains here vs the previous iteration.

Iteration 7

If there's one thing I know about JavaScript, it's that property access is fast. seen doesn't need to be an array, what if we just kept a dictionary of seen keys?

function uniqueBy(arr, prop){
  const len = arr.length
  let cursor = 0
  const record = []
  const seen = {}
  while (cursor < len) {
    const item = arr[cursor]
    const val = item[prop]
    if (!seen[val]) {
      seen[val] = 1
      record.push(item)
    }
    cursor++
  }
  return record
}

Result: 24,970 ops/sec

The best one yet!

Iteration 8

Okay after doing some more research on loops, I came across this little number

function uniqueBy(arr, prop){
  const record = []
  const seen = {}
  for (let i = 0, len = arr.length; i < len; ++i) { // Notice the len = arr.length
    const item = arr[i]
    const val = item[prop]
    if (!seen[val]) {
      seen[val] = 1
      record.push(item)
    }
  }
}

Result: 26,390 ops/sec

Hmm, this is the winner (so far). But why? Didn't we find the while loop faster? All that's happening is the len = array.length is just caching the length. We were already doing that?

All I can think is happening is something to do with Locality of Reference. I have no formal Computer Science, and I'm not a particularly smart man. If someone can explain to me why this is faster, please comment 😅

I recreated these tests on ESBench here: ESBench Results if that's more your cup of tea.

Bonus

Here are some other variations I tested with negligible performance gains/losses:

++cursor vs cursor++

function uniqueBy(arr, prop) {
  const len = arr.length
  let cursor = -1
  const record = []
  const seen = []
  while (++cursor < len) {
    const item = arr[cursor]
    const val = item[prop]
    if (!~seen.indexOf(val)) {
      seen.push(val)
      record.push(item)
    }
  }
  return record
}

Reducing variables (💩 )

function uniqueBy(arr, prop) {
  const len = arr.length
  let cursor = -1
  const record = []
  const seen = []
  while (++cursor < len) {
    if (!~seen.indexOf(arr[cursor][prop])) {
      seen.push(arr[cursor][prop])
      record.push(arr[cursor])
    }
  }
  return record
}

Summary

This whole process is mostly a fruitless endeavour. We could have stopped at iteration number 3 and put our feet up; however, I just wanted to see how fast we could make it. I'm glad I carried on since I found the seen object approach.

You do not need to do this in your applications. You should only go this deep down the rabbit hole (and arguably further), if you're experiencing performance issues.

If you have a faster way, please ping me on Twitter @moistmakerr or comment. I'd love to know how fast we can push this.