Against closure consts

Imagine that your a developing a library that you want to open-source or you created a helper that will be used all over the codebase in a mid-to-large project. Your little library exposes a function that does what is intented but relies on some (sensible in your eyes) const values. Lets imagine that our library is compressing a file in 3 different levels (high compression, mid compression, low compression).

Initially you have something like:

export function compressFile(file){
  const compressedFiles = [];
  for(let level in [90, 50, 30]){
    // compress with compressionLevels[level]
    // and compressedFiles.push(compressedFile)
  }

  return compressedFiles;
}

This [90, 50, 30] look like a magic value, so you refactor a bit to make it more clear and you end up with something like this:

const compressionLevels = [90, 50, 30];

export function compressFile(file){
  const compressedFiles = [];
  for(let level in compressionLevels){
    // compress with compressionLevels[level]
    // and compressedFiles.push(compressedFile)
  }

  return compressedFiles;
}

There are quite a few problems with this approach:

  • If you open source this function (or even use it internally in a large codebase), you can’t tweak these values anymore without “breaking” the product of the people who use the function. Imagine that you change the 90 to 100 to offer even better compression, but as a side effect your function is significantly slower now. Anyone who relies on your function will have to pay that cost.
  • The people who use your function doest know that you rely on some const and that they could potentionally tweak these values but you dont offer them this opportunity.
  • Testing is less reliable, I supposed you have tests that cover your hardcoded values but you cant really do boundary testing (e.g. that the low compression fails if value is 29 instead of 30)

When we design a function that we know it will be used outside of the file we are, we should no rely in const defined in the closure of it but prefer to explicitly pass it:

export function compressFile(file, compressionLevels){
  const compressedFiles = [];
  for(let level in compressionLevels){
    // compress with compressionLevels[level]
    // and compressedFiles.push(compressedFile)
  }

  return compressedFiles;
}

If we know that 90% of the time the values that we had picked are reasonable we can define them as defaults:

export function compressFile(file, compressionLevels = [90, 50, 30]){
  const compressedFiles = [];
  for(let level in compressionLevels){
    // compress with compressionLevels[level]
    // and compressedFiles.push(compressedFile)
  }

  return compressedFiles;
}

And ofcourse validate that the values passed by the caller have the right type/shape:

export function compressFile(file, compressionLevels = [90, 50, 30]){
  if(hasValidCompressionLevels(compressionLevels)){
    throw Error('Cannot compress file, compressionLevels param should be Array<number>')
  }

  const compressedFiles = [];
  for(let level in compressionLevels){
    // compress with compressionLevels[level]
    // and compressedFiles.push(compressedFile)
  }

  return compressedFiles;
}
Published 13 Nov 2020

Engineering Manager. Opinions are my own and not necessarily the views of my employer.
Avraam Mavridis on Twitter