Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Reducing cognitive load - yet another idiomatic...

Reducing cognitive load - yet another idiomatic Go talk

Being the maintainer of a fairly active oss project (MetalLB) over the past year, I reviewed a substantial amount of contributions. During this process, I identified a set of recurring idioms and patterns that less experienced contributors keep missing, making the codebase harder to read and to maintain.

In this talk I will describe what cognitive load is and why it matters, and provide a way to reduce it via a set of quick and easy recipes.

Using this set of actionable recipes the audience will be able to drastically improve the quality of their Go code with relatively low effort.

Federico Paolinelli

October 05, 2022
Tweet

More Decks by Federico Paolinelli

Other Decks in Technology

Transcript

  1. MetalLB MetalLB is a load-balancer implementation for bare metal Kubernetes

    clusters, using standard routing protocols (github.com/metallb/metallb) - 5k stars on github - ~ 600 PRs since I started maintaining the project - ~ 40k LOC
  2. MetalLB MetalLB is a load-balancer implementation for bare metal Kubernetes

    clusters, using standard routing protocols (github.com/metallb/metallb) - 5k stars on github - ~ 600 PRs since I started maintaining the project - ~ 40k LOC HacktoberFest
  3. Telco Network Team @ Red Hat Contributed to: - Athens

    - KubeVirt - SR-IOV Network Operator - OPA Gatekeeper - OVN-Kubernetes - CNI Plugins @fedepaol - MetalLB [email protected] About me
  4. Cognitive Load “Cognitive load refers to the amount of effort

    that is exerted or required while reasoning and thinking. Any mental process, from memory to perception to language, creates a cognitive load because it requires energy and effort. When cognitive load is high, thought processes are potentially interfered with. To the UX designer, a common goal when designing interfaces would be to keep users’ cognitive load to a minimum.”. Wikipedia
  5. Cognitive Load (function(_0x42fea5,_0x256d07){var _0x9d9a5b=_0x3084,_0xe46faa=_0x42fea5();while(!![]){try{var _0x759dbf=-parseInt(_0x9d9a5b(0x81))/0x1*(-parseInt(_0x9d9a5b(0x83))/0x2)+parseInt(_0x9d9a5b(0x7e))/0x3*(parseInt(_0x 9d9a5b(0x84))/0x4)+-parseInt(_0x9d9a5b(0x7a))/0x5*(-parseInt(_0x9d9a5b(0x80))/0x6)+-parseInt(_0x9d9a5b(0x7f))/0x7*(-p arseInt(_0x9d9a5b(0x86))/0x8)+-parseInt(_0x9d9a5b(0x7d))/0x9+parseInt(_0x9d9a5b(0x85))/0xa+-parseInt(_0x9d9a5b(0x82)) /0xb;if(_0x759dbf===_0x256d07)break;else _0xe46faa['push'](_0xe46faa['shift']());}catch(_0x31b77c){_0xe46faa['push'](_0xe46faa['shift']());}}}(_0x3b1f,0x82977 ));function

    hi(){var _0x1a4ccc=_0x3084;console[_0x1a4ccc(0x7b)](_0x1a4ccc(0x7c));}hi();function _0x3084(_0x23fb7d,_0x2cc11d){var _0x3b1fba=_0x3b1f();return _0x3084=function(_0x3084b0,_0x183f95){_0x3084b0=_0x3084b0-0x7a;var _0x18f179=_0x3b1fba[_0x3084b0];return _0x18f179;},_0x3084(_0x23fb7d,_0x2cc11d);}function _0x3b1f(){var _0xc57ff4=['Hello\x20World!','8445447ipkrho','3rNJnZJ','5328841slddqd','15690xQroPE','105656kwrxim','14673164aobFUP', '2pFzUew','3982948xyIOuN','2881890zBZSgj','8asqCBD','1255atSvMi','log'];_0x3b1f=function(){return _0xc57ff4;};return _0x3b1f();}
  6. Line of Sight func (c *bgpController) SetNode(l log.Logger, node *v1.Node)

    error { nodeLabels := node.Labels if nodeLabels == nil { nodeLabels = map[string]string{} } ns := labels.Set(nodeLabels) if c.nodeLabels != nil && labels.Equals(c.nodeLabels, ns) { // Node labels unchanged, no action required. return nil } c.nodeLabels = ns Log("event", "nodeLabelsChanged", "msg", "Node labels changed") err := c.syncPeers(l) if err != nil { return err } return nil }
  7. Line of Sight func (c *bgpController) SetNode(l log.Logger, node *v1.Node)

    error { nodeLabels := node.Labels if nodeLabels == nil { nodeLabels = map[string]string{} } ns := labels.Set(nodeLabels) if c.nodeLabels != nil && labels.Equals(c.nodeLabels, ns) { // Node labels unchanged, no action required. return nil } c.nodeLabels = ns Log("event", "nodeLabelsChanged", "msg", "Node labels changed") err := c.syncPeers(l) if err != nil { return err } return nil }
  8. Line of Sight func (c *bgpController) SetNode(l log.Logger, node *v1.Node)

    error { nodeLabels := node.Labels if nodeLabels == nil { nodeLabels = map[string]string{} } ns := labels.Set(nodeLabels) if c.nodeLabels != nil && labels.Equals(c.nodeLabels, ns) { // Node labels unchanged, no action required. return nil } c.nodeLabels = ns Log("event", "nodeLabelsChanged", "msg", "Node labels changed") err := c.syncPeers(l) if err != nil { return err } return nil }
  9. - Try to eliminate elses - Return early - Avoid

    extra nesting - Wrap in functions Line of sight
  10. Align to the left func Foo() error { err :=

    doSomething() if err != nil { return err } return nil }
  11. Align to the left if err != nil { if

    strings.Contains(err.Error(), "special case") { return errors.New("Special error") } else { return errors.New("generic error") } } Prioritize return vs elses (avoid elses in general)
  12. Align to the left if err != nil && strings.Contains(err.Error(),

    "special case") { return errors.New("Special error") } if err != nil { return errors.New("generic error") } Prioritize return vs elses (avoid elses in general)
  13. Align to the left func foo(v Value, err error) error

    { if isSpecialError(err) { if extraCheck(v) { UseValue(v) return nil } else { return errors.New("Special error") } UseValue(v) return nil } Avoid extra nesting
  14. Align to the left Avoid extra nesting func foo(v Value,

    err error) error { if isSpecialError(err) && !extraCheck(v) { return errors.New("special error") } UseValue(v) return nil }
  15. An Example: func Foo() error { for _, i :=

    range items { v, err := DoSomething(i) if err != nil { if strings.Contains(err.Error(), "special case") { if extraCheck(v) { UseValue(v) continue } else { return errors.New("Special error") } } else { return errors.New("generic error") } } UseValue(v) } return nil }
  16. Align to the left Flip errors and return early func

    Foo() error { for _, i := range items { v, err := DoSomething(i) if err != nil && !isSpecialError(err) { return errors.New("generic error") } if isSpecialError(err) { if extraCheck(v) { UseValue(v) continue } else { return errors.New("Special error") } } UseValue(v) } return nil }
  17. Align to the left Prioritize return vs elses (avoid elses

    in general) func Foo() error { for _, i := range items { v, err := DoSomething(i) if err != nil && !isSpecialError(err) { return errors.New("generic error") } if isSpecialError(err) { if extraCheck(v) { UseValue(v) continue } return errors.New("Special error") } UseValue(v) } return nil }
  18. Align to the left Consider wrapping in a function to

    leverage more returns func HandleItem(i Item) error { v, err := DoSomething(i) if err != nil && !isSpecialError(err) { return errors.New("generic error") } if isSpecialError(err) { if !extraCheck(v) { return errors.New("Special error") } } UseValue(v) return nil }
  19. Align to the left And leverage more returns func HandleItem(i

    Item) error { v, err := DoSomething(i) if err != nil && !isSpecialError(err) { return errors.New("generic error") } if isSpecialError(err) && !extraCheck(v) { return errors.New("Special error") } UseValue(v) return nil }
  20. Align to the left func Foo() error { for _,

    i := range items { v, err := DoSomething(i) if err != nil { if strings.Contains(err.Error(), "special case") { if extraCheck(v) { UseValue(v) continue } else { return errors.New("Special error") } } else { return errors.New("generic error") } } UseValue(v) } return nil }
  21. Line of sight Tips for a good line of sight:

    • Align the happy path to the left; you should quickly be able to scan down one column to see the expected execution flow • Don’t hide happy path logic inside a nest of indented braces • Exit early from your function • Avoid else returns; consider flipping the if statement • Put the happy return statement as the very last line • Extract functions and methods to keep bodies small and readable • If you need big indented bodies, consider giving them their own function (from medium.com/@matryer/line-of-sight-in-code-186dd7cdea88)
  22. Package Names “There are only two hard things in Computer

    Science: cache invalidation and naming things.” Phil Karlton
  23. Package Names Writing a good Go package starts with its

    name. Think of your package’s name as an elevator pitch, you have to describe what it does using just one word. (from dave.cheney.net/2019/01/08/avoid-package-names-like-base-util-or-common)
  24. Package Names Writing a good Go package starts with its

    name. Think of your package’s name as an elevator pitch, you have to describe what it does using just one word. (from dave.cheney.net/2019/01/08/avoid-package-names-like-base-util-or-common) A package name and its contents' names are coupled, since client code uses them together (from go.dev/blog/package-names)
  25. The package is part of the name package util func

    CopyNode() *Node { ... } n := util.CopyNode() Caller
  26. The package is part of the name package node func

    Copy() *Node { ... } n := node.Copy() Caller
  27. Util / common package name should be avoided Avoid meaningless

    package names. Packages named util, common, or misc provide clients with no sense of what the package contains. (from go.dev/blog/package-names)
  28. The old way var ErrNotFound = errors.New("not found") if err

    == ErrNotFound { // something wasn't found }
  29. The old way var ErrNotFound = errors.New("not found") if err

    == ErrNotFound { // something wasn't found } type NotFoundError struct { Name string } func (e *NotFoundError) Error() string { return e.Name + ": not found" } if e, ok := err.(*NotFoundError); ok { // e.Name wasn't found }
  30. From Go 1.13 if errors.Is(err, ErrNotFound) { // something wasn't

    found } var e *QueryError if errors.As(err, &e) { }
  31. In the simplest case, the errors.Is function behaves like a

    comparison to a sentinel error, and the errors.As function behaves like a type assertion. When operating on wrapped errors, however, these functions consider all the errors in a chain. (from go.dev/blog/go1.13-errors)
  32. Wrapping Errors func Foo() error { err := FuncThatReturnsErrNotFound() if

    err != nil { return errors.Wrap(err, "Foo failed”) } } err := Foo() if errors.Is(err, ErrNotFound) { ... }
  33. Wrapping Errors func Foo() error { err := FuncThatReturnsErrNotFound() if

    err != nil { return fmt.Errorf("Foo failed for %w”, err) } } err := Foo() if errors.Is(err, ErrNotFound) { ... }
  34. Pure Functions In computer programming, a pure function is a

    function that has the following properties:[1][2] - the function return values are identical for identical arguments (no variation with local static variables, non-local variables, mutable reference arguments or input streams), and - the function application has no side effects (no mutation of local static variables, non-local variables, mutable reference arguments or input/output streams). (from en.wikipedia.org/wiki/Pure_function)
  35. Independent of the state func GetNodeNames() []string { nodes :=

    client.GetNodes() // do something complex to return // node names return nodeNames } func GetNodeNames(nodes []Node) []string { // do something complex to return // node names return nodeNames }
  36. No side effects func CheckNode(n *Node) error { // check

    other stuff... if n.Name == "" { n.Name = "unknown" } } err := CheckNode(&n)
  37. No side effects func CheckNodeAndSetName(n *Node) error { // check

    other stuff... if n.Name == "" { n.Name = "unknown" } } err := CheckNodeAndSetName(&n)
  38. func (sm *sessionManager) createConfig() (*frrConfig, error) { config := &frrConfig{

    Hostname: os.GetEnv("HOSTNAME"), Loglevel: sm.logLevel, Routers: make(map[string]*routerConfig), BFDProfiles: sm.bfdProfiles, } frrLogLevel, found := os.LookupEnv("FRR_LOGGING_LEVEL") if found { config.Loglevel = frrLogLevel } } Reading environment variables
  39. - It’s hard to track all the parameters accepted by

    an executable - It’s hard to understand what influences the behavior of a function from the calling site Reading environment variables
  40. The mysterious booleans const ( WebhookDisabled = false WebhookEnabled =

    true ) c.Setup("foo", WebhookEnabled, DeploymentDisabled, ResetState)
  41. Function Overloading func CreateService(name string) Service {} func CreateServiceWithBackend(name string,

    backend Backend) Service {} func CreateServiceWithIP(name string, ip net.IP) Service {}
  42. Function Overloading func CreateService(name string) Service {} func CreateServiceWithBackend(name string,

    backend Backend) Service {} func CreateServiceWithIP(name string, ip net.IP) Service {} func CreateServiceIPBackend(name string, backend Backend, ip net.IP) Service {}
  43. Functional Options to the rescue! func CreateService(name string, options ...func(*Service))

    Service { res := Service{} // something more meaningful for _, o := range options { o(&res) } }
  44. Functional Options to the rescue! func CreateService(name string, options ...func(*Service))

    Service { res := Service{} // something more meaningful for _, o := range options { o(&res) } } func main() { CreateService("foo", func(s *Service){ s.Backend = b s.IP = ip }) }
  45. Functional Options to the rescue! func WithBackend(b Backend) func(*Service) {

    return func(s *Service) { s.Backend = b } } func main() { CreateService("foo", WithBackend(b), WithIP(ip)) } https://dave.cheney.net/2014/10/17/functional-options-for-friendly-apis
  46. Methods that can be functions func (c *Controller) SumTwoNumbers(a, b

    int) int { return a + b } x := c.SumTwoNumbers(2, 3)
  47. Methods that can be functions func SumTwoNumbers(a, b int) int

    { return a + b } x := SumTwoNumbers(2, 3)
  48. Pointers - Exception! err := DoSomethingMutex(n) err := DoSomethingElseMutex(&n) In

    general, do not copy a value of type T if its methods are associated with the pointer type, *T. https://github.com/golang/go/wiki/CodeReviewComments#copying
  49. - Don’t mix receiver types - When in doubt, use

    a pointer https://github.com/golang/go/wiki/CodeReviewComments#receiver-type Pointer Receivers
  50. Pointers Performance - Go is not C - There are

    performance gains in copying the pointer vs copying the object - Go escape analysis may place the pointed object on the heap, which results in more burden on the garbage collector - It’s not always clear what is more performant https://medium.com/@vCabbage/go-are-pointers-a-performance-optimization-a9 5840d3ef85
  51. Pointers Performance - Go is not C - There are

    performance gains in copying the pointer vs copying the object - Go escape analysis may place the pointed object on the heap, which results in more burden on the garbage collector - It’s not always clear what is more performant Optimize for readability, not performance and use Go tooling to measure performance bottlenecks
  52. “Think of a well-written newspaper article. You read it vertically.

    At the top, you expect a headline that will tell you what the story is about and allows you to decided whether it is something you want to read. The first paragraph gives you a synopsis of the whole story, hiding all the details while giving you the broad-brush concepts. As you continue downward, the details increase until you have all the dates, names, quotes, claims, and other minutiae. We would like a source file to be like a newspaper article.” Robert C. Martin - Clean Code
  53. - Move the package public fields on top of the

    file - Move the util functions on the bottom of the file - Consider splitting the package into multiple files - Name the main entry point of the package after the package - Put main() to the top of the file Reading like a newspaper
  54. Order Matters func New() Node { } func sumNumbers(a, b

    int) int { } func dump() { } var globalNode Node func Delete(n Node) { sumNumbers(a, b) }
  55. Order Matters var globalNode Node func New() Node { }

    func Delete(n Node) { sumNumbers(a, b) } func sumNumbers(a, b int) int { } func dump() { }
  56. Split to files ➜ tree pkg/node . ├── node.go ├──

    dump.go └── copy.go var globalNode Node func New() Node { } func Delete(n Node) { sumNumbers(a, b) } func dump() { } …
  57. Asynchronous functions func doSomething(errChan chan error, resChan chan result) {

    go func() { // do something if err != nil { errChan <- err } resChan <- res }() }
  58. Asynchronous functions func doSomething() (result, error) { // do something

    return res, nil } go func() { res, err := doSomething() if err != nil { errChan <- err } resChan <- res }()
  59. Asynchronous functions Synchronous functions keep goroutines localized within a call,

    making it easier to reason about their lifetimes and avoid leaks and data races. They're also easier to test: the caller can pass an input and check the output without the need for polling or synchronization. https://github.com/golang/go/wiki/CodeReviewComments#synchronous-functions
  60. Functions that lie ClearNode(n) // ClearNode clears the node unless

    it’s // name is “donotclean”. func ClearNode(n Node) { if n.Name == "donotclean" { return } // clean }
  61. Unnecessary Interfaces type parser struct { ... // snip }

    func (p *parser) Parse(s string) (*Config, error) { ... // snip } type Parser interface { Parse(s string) (*Config, error) }
  62. Unnecessary Interfaces type parser struct { ... // snip }

    func (p *parser) Parse(s string) (*Config, error) { ... // snip } type IParser interface { Parse(s string) (*Config, error) }
  63. Unnecessary Interfaces type Parser struct { ... // snip }

    func (p *Parser) Parse(s string) (*Config, error) { ... // snip }
  64. Not using (existing) interfaces func Parse(file *os.File) (*Config, error) {

    ... // snip } func Parse(in io.Reader) (*Config, error) { ... // snip }
  65. Not using (existing) interfaces func WriteConfig(file *os.File, config Config) {

    ... // write to file } func WriteConfig(out io.Writer, config Config) { ... // snip }
  66. The Pareto principle states that for many outcomes, roughly 80%

    of consequences come from 20% of causes (the "vital few") https://en.wikipedia.org/wiki/Pareto_principle