7

When it comes to if statements it is possible to refactor this code

(This is just an example and does not refer to the "real" code)

if(person === 'customer' || person === 'employee' || person === 'other')

to

if(person === ('customer' || 'employee' || 'other'))

Currently I have an object called state containing 3 boolean properties. I want to show an overlay if at least one property returns true. My current solution is this

showOverlay: state => state.isNavigating || state.isHttpRequesting || state.isProcessing

and I'm asking if there is a way to clean it up. Pseudo code would be

showOverlay: state => (isNavigating || isHttpRequesting || isProcessing) of state

I know there is not a big gain out of it, but it would remove all the state. ... parts.

  • 1
    `it is possible to refactor this code` no, it isn't, they are two different results – Jaromanda X Apr 10 '19 at 09:42
  • `['customer', 'employee', 'other'].includes(person)` but that's quite different from your other code – CertainPerformance Apr 10 '19 at 09:42
  • 1
    @CertainPerformance - OP states that the second piece of code **IS** valid refactoring of the first! – Jaromanda X Apr 10 '19 at 09:43
  • `state.isNavigating || state.isHttpRequesting || state.isProcessing` is very readable. You could probably destructure the properties and then use OR but that's an additional step – adiga Apr 10 '19 at 09:45
  • `This is just an example and does not refer to the "real" code` - that's not what you said, you said the first two pieces of code are the same – Jaromanda X Apr 10 '19 at 09:46
  • 1
    `state => { with(state) { return isNavigating || isHttpRequesting || isProcessing; } }` ... just kidding :) – Jonas Wilms Apr 10 '19 at 09:47
  • "is there a way to clean it up" ... to be honest: the code is actually really clean, what else do you want :) – Jonas Wilms Apr 10 '19 at 09:50
  • 1
    I'm sorry but `premature optimization is the root of all evil` is my biggest enemy for now .. :( –  Apr 10 '19 at 09:51

1 Answers1

4

Use an array of property names, and check whether some of them are truthy:

showOverlay: state => ['isNavigating', 'isHttpRequesting', 'isProcesing'].some(prop => state[prop]);

Maybe it's more DRY, which is arguably good, but it's a bit less readable when there are only 3 properties IMO, which is arguably bad.

CertainPerformance
  • 356,069
  • 52
  • 309
  • 320
  • thanks, what about performance? Is it bad to create an array and loop through it? –  Apr 10 '19 at 09:48
  • 2
    On modern computers, performance is almost never something to worry about unless you've run a performance test and have identified a section of code causing a bottleneck - don't worry about it. Clean, readable code is better by default. (though, is this clean and readable? Up to you. Maybe if there were 4+ properties, I would definitely prefer the array, but I'm not so sure if there are only 3) – CertainPerformance Apr 10 '19 at 09:49