Skip to content

Instantly share code, notes, and snippets.

@willfrew
Created September 16, 2020 08:04
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save willfrew/bccfa7601688ee7490db6c0cb0bd1a77 to your computer and use it in GitHub Desktop.
Save willfrew/bccfa7601688ee7490db6c0cb0bd1a77 to your computer and use it in GitHub Desktop.
Getters

The Problem with get

Ever since the get and set keywords came to Javascript / Typescript, I've felt uneasy about them. Something about their nature of hiding computation in what appears to be data never really sat well with me. In some projects, I knee-jerk banned them with lint rules but unfortunately I never really thought deeply about why.

Let's start with some assertions:

  1. When you're reading through a piece of code, you are mentally executing the program. Symbolically at first and with specific inputs while debugging.
  2. When you're reading through a piece of code that is new to you, in order to read and mentally execute a piece of code without reading the entire codebase, you need to be able rely on interfaces & type definitions.
  3. When working in a team, it is natural that you will frequently arrive at parts of the codebase that you have not worked on before or have been modified since you last read the code. As the codebase and team grows, this becomes more and more frequent.
  4. In order for team members to continue to read and write code at a good rate, it is important to minimise the complexity/difficulty of mentally executing parts of the codebase.

Since getters and seters are features of class syntax, we will for now assume that we are using mutable objects* (as opposed to immutable objects) freely in our hypothetical codebase. It's certainly possible to write classes that function as immutable objects and do all their computations purely, but, in my experience, it's not common in codebases making use of OOP.

* The use of mutable data generally also implies impure computation (as opposed to pure computation) since we need to mutate the data somehow.

Following from the previous assertions:

  1. In the presence of mutable objects, you must take extra care when mentally executing code to take into account that performing some computation -- before and after doing some state mutation -- may yield different results with the same apparent inputs.
  2. Every time you see some state mutation, you need to update your mental model of the current state of the program and any later computations that create derived data from the new state of the program need to be mentally re-executed.

Thinking specifically about line-by-line execution, rather than at the software-architectural level; while you're executing the program, you need to maintain the (relevant) current state in your mind and track how it evolves after each line.

On each line, you can have reads and writes and you need to keep track of what is being written to, what is being read and in what order.

Writes tend to be explicit assignments or methods whose name (should) make it clear that they perform some write and to where.

Reads are variable usages, property references, or method calls that like-wise should make it clear that they are just reading (and not writing).

For each write, you need to update your mental model of the program state. For reads, you can read an existing value from your mental model or execute methods calls to calculate and produce new bits of state. Method calls (that return values) are therefore writing to your mental program state since they compute values that were not there before.

From this, we can see that:

  1. In the presence of getters, you have no way of knowing whether the object your function has been passed has plain data properties and is part of the program state to be mentally tracked or if it is derived from other parts of the state and is a computation that needs re-executing after writes occur elsewhere. Unless, you go and read the 'implementation' of the transitive closure of your code's dependencies.
  2. If you haven't read the 'implementation' of all the dependencies to see if there are any getters in there and what they are derived from, you can unknowingly modify one of its dependencies and its value can fluctuate 'between' the lines of code you are reading without you realising that you needed to update its value in your mental model.
  3. In a codebase with a mixture of plain values and getters you must go and read the implementation of every single data property you are using to check if it's a getter or not.

This is bad for a several reasons:

  • You have pushed the burden of knowledge of the implementation of interface onto the consumer of the API.
  • You have increased the number of things you need to mentally re-execute over and over again massively.
  • You now cannot rely on language syntax anymore to tell the difference between data and computation.

Put another way: In the presence of mutable data, you generally need to assume that all methods/function calls need to be re-executed if the program state changed. However, in the presence of mutable data and getters, everything might need to be re-executed mentally. You have no way of knowing. You have to go and check.

Looking at some examples from the other side of this, if we mix usage of getters and non-getters in our codebase, how can we know whether a function we want to use expects plain data properties or dynamically updated ones (implemented with a getter) without reading its implementation? Consider the following example:

type Data = {
  numbers: number[];
  isEmpty: boolean;
};

const sendNumbers = (data: Data): void => {
  while (!data.isEmpty) {
    send(data.numbers.pop());
  }
};

The sendNumbers function works fine if data is implemented as follows:

sendNumbers({
  numbers: [1, 2, 3, 4, 5],
  get isEmpty() {
    this.numbers.length === 0;
  }
});

But the type definition does not say anything about whether isEmpty (or even numbers) re-compute their value or not on each read and an unwitting consumer of the API can just as easily pass the following with no complaints from the type system:

sendNumbers({
  numbers: [1, 2, 3, 4, 5],
  isEmpty: false, // Infinite loop
});
sendNumbers({
  get numbers() {
    return /* calculate a new array from some other dependencies */; // Infinite loop
  },
  get isEmpty() {
    this.numbers.length === 0;
  }
});

Now these specific examples would almost certainly get caught while writing/performing tests but the range of possible bugs is essentially infinite and depends on the application. The core problem stays the same:

When using a mixture of getters and plain data properties, we can no longer depend on language syntax and interface contracts to effectively reason about what causes which parts of the program state to change.

@toteto-h4
Copy link

Generally I agree that having computation hidden in a getter for a field it is bit strange and unexpected.

But instead of having computed fields we have plain data fields that stores that value is causing more harm and is in no way the solution. Lets imagine the scenario with the sendNumbers. If the Data implementation would listen for each change to the numbers array and with that modify the some isEmpty: boolean field, the Data implementation has to take care of this state in every place that the numbers array is changed. This can be one place or multiple ones.

class Data {
  numbers = [1, 2, 3, 4, 5];
  isEmpty = numbers.length === 0

  constructor() {
    numbers.onAdd(() => (isEmpty = numbers.length === 0))
    numbers.onRemove(() => (isEmpty = numbers.length === 0))
  }
}

Conclusion: if a state can be derived/computed based on other state, storing that computed state in some field is pretty bad decision.


On the other hand only reason why i would prefer getter instead of accesor method (isEmpty(data) or data.isEmpty() is the lazy computation and memoization MobX gives us by using @computed on the getter itself. Not sure how MobX will behave if we were to transform the getter in accesor method.

This being said, I am completely OK and would prefer if we were to use methods instead of getters, but only if we are OK without the performance benefit mobx gives us by using @computed get x() {}.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment