Replace function registry pattern
- Dominant language
- Go
- Stars
- 30.1k
- Forks
- 4.6k
- Avg merge
- 1d 18h
- Merged PRs (30d)
- 39
Description
There are a number of packages (see below for the list) where we use a registry to store functions in a map. In 05d5ec480497a0b0952eba2de5305e611cfb6d73 we replaced one of those instances with a different pattern. The new pattern was simply a function that accepted a list of other functions.
The new pattern of simply calling functions directly has a few advantages over the "function registry" pattern:
* it is much easier to trace the code. The function registry pattern requires many hops (init -> register -> map definition -> map usage), with a direct function call it is simply one hop (function -> caller).
* it is easier to write a reasonable test when the code being tested accepts its dependencies as inputs, instead of them being declared statically.
* in some cases this pattern forces us to keep all the functions in the same package, or even as methods on the same struct (because the registry is of unbound methods instead of functions). This leads to very large packages and sub-optimal code layout which cause significant developer productivity issues.
This issue proposes we addresses these problems by replacing each of these instances of the "function registry" pattern with direct function calls, similar to 05d5ec480497a0b0952eba2de5305e611cfb6d73.
We can see this pattern in the following places:
* CLI commands - `command/commands_oss.go`
* HTTP handlers - `agent/http_register.go`
* FSM operations - `agent/consul/fsm/commands_oss.go`
* FSM snapshot persist functions - `agent/consul/fsm/snapshot_oss.go`
* FSM snapshot restore functions - `agent/consul/fsm/snapshot_oss.go`
* ACL auth methods - `agent/consul/authmethod`
* RPC handlers - `agent/consul/server_register.go`
* Watch functions - `api/watch/funcs.go`
Contributor guide
Research direction
Start by reading commit 05d5ec480497a0b0952eba2de5305e611cfb6d73, then compare the registry patterns in command/commands_oss.go, agent/http_register.go, agent/consul/fsm/commands_oss.go, agent/consul/fsm/snapshot_oss.go, agent/consul/authmethod, agent/consul/server_register.go, and api/watch/funcs.go. Done means each listed registry instance has been replaced with direct function calls and the affected behavior remains covered by the project’s tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- api, backend, cli, distributed-systems
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 30/100