Kevin Whitaker Kevin Whitaker -4 years ago 65
Javascript Question

Javascript: How can I replace nested if/else with a more functional pattern?

The following pattern gets repeated in my React app codebase quite a bit:

const {items, loading} = this.props
const elem = loading
? <Spinner />
: items.length
? <ListComponent />
: <NoResults />


While this is certainly cleaner than nesting actual
if/else
clauses, I'm trying to embrace more elegant and functional patterns. I've read about using something like an
Either
monad, but all of my efforts towards that have ended up looking more verbose, and less reusable (this pseudo-code probably doesn't work, given that I'm trying to remember previous attempts):

import {either, F, isEmpty, prop} from 'ramda'
const isLoading = prop('loading')
const renderLoading = (props) => isLoading(props) ? <Spinner /> : false
const loadingOrOther = either(renderLoading, F)
const renderItems = (props) => isEmpty(props.items) ? <NoResults /> : <ListComponent />
const renderElem = either(loadingOrOther, renderItems)
const elems = renderElem(props)


What pattern can I use that would be more DRY/reusable?

Thanks!

Answer Source

While this is certainly cleaner than nesting actual if/else clauses

render () {
  const {items, loading} = this.props
  return loading
    ? <Spinner />
    : items.length
      ? <ListComponent items={items} />
      : <NoResults />
}

You've posted incomplete code, so I'm filling in some gaps for a more concrete example.

Looking at your code, I find it very difficult to read where conditions are, and where return values are. Conditions are scattered across various lines at various indentation levels – likewise, there is no visual consistency for return values either. In fact it's not apparent that loading in return loading is even a condition until you read further into the program to see the ?. Choosing which component to render in this case is a flat decision, and the structure of your code should reflect that.

Using if/else produces a very readable example here. There is no nesting and you get to see the various types of components that are returned, neatly placed next to their corresponding return statement. It's a simple flat decision with a simple, exhaustive case analysis.

I stress the word exhaustive here because it is important that you provide at minimum the if and else choice branches for your decision. In your case, we have a third option, so one else if is employed.

render () {
  const {items, loading} = this.props
  if (loading)
    return <Spinner />
  else if (items.length)
    return <ListComponent items={items} />
  else
    return <NoResults />
}

If you look at this code and try to "fix" it because you're thinking "embrace more elegant and functional patterns", you misunderstand "elegant" and "functional".

There is nothing elegant about nested ternary expressions. Functional programming isn't about writing a program with the fewest amount of keystrokes, resulting in programs that are overly terse and difficult to read.

if/else statements like the one I used are not somehow less "functional" because they involve a different syntax. Sure, they're more verbose than ternary expressions, but they operate precisely as we intend them to and they still allow us to declare functional behaviour – don't let syntax alone coerce you into making foolish decisions about coding style.

I agree it's unfortunate that if is a statement in JavaScript and not an expression, but that's just what you're given to work with. You're still capable of producing elegant and functional programs with such a constraint.


Remarks

I personally think relying upon truthy values is gross. I would rather write your code as

render () {
  const {items, loading} = this.props
  if (loading)                              // most important check
    return <Spinner />
  else if (items.length === 0)              // check of next importance
    return <NoResults />
  else                                      // otherwise, everything is OK to render normally
    return <ListComponent items={items} />
}

This is less likely to swallow errors compared to your code. For example, pretend for a moment that somehow your component had prop values of loading={false} items={null} – you could argue that your code would gracefully display the NoResults component; I would argue that it's an error for your component to be in a non-loading state and without items, and my code would produce an error to reflect that: Cannot read property 'length' of null.

This signals to me that a bigger problem is happening somewhere above the scope of this component – ie this component has either loading=true or some array of items (empty or otherwise); no other combination of props is acceptable.

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download