0

So, I have these functions:

const myFunc1 = () => async (dispatch, getState) => {
  if (await dispatch(isSessionExpired())) return;

  const authToken = getAuthToken(getState());
  if(!authToken){
    dispatch(statuses.myFunc1Error());
    return;
  }

  //do something 
}

const myFunc2 = () => async (dispatch, getState) => {
  if (await dispatch(isSessionExpired())) return;

  const authToken = getAuthToken(getState());
  if(!authToken){
    dispatch(statuses.myFunc2Error());
    return;
  }

  //do something 
}

const myFunc3 = () => async (dispatch, getState) => {
  if (await dispatch(isSessionExpired())) return;

  const authToken = getAuthToken(getState());
  if(!authToken){
    dispatch(statuses.myFunc3Error());
    return;
  }

  //do something 
}

As you can see, there's a lot of repeated code, what's the best way to refactor this? I though doing something like this:

const creatorChecks = async (dispatch, getState, actionName) => {
    if (await dispatch(isSessionExpired())) return false;

    const authToken = getAuthToken(getState());
    if(!authToken){
      dispatch(statuses[`${actionName}Error`]());
      return false;
    }

    return true;
}

and then...

const myFunc1 = () => async (dispatch, getState) => {
  if(!await creatorChecks(dispatch, getState, 'myFunc1')) return;
  // the code...
}

const myFunc2 = () => async (dispatch, getState) => {
  if(!await creatorChecks(dispatch, getState, 'myFunc2')) return;
  // the code...
}

const myFunc3 = () => async (dispatch, getState) => {
  if(!await creatorChecks(dispatch, getState, 'myFunc3')) return;
  // the code...
}

But maybe there's a better way...

nick
  • 2,819
  • 5
  • 33
  • 69

1 Answers1

0

Extract your common logic into a function and take the error producer and actual code to use as callbacks. That gives you a common algorithm for all with interchangeable parts of the implementation:

const decorate = errFn => fn => () => async (dispatch, getState) => {
  if (await dispatch(isSessionExpired())) return;

  const authToken = getAuthToken(getState());
  if(!authToken){
    dispatch(errFn());
    return;
  }

  return fn(dispatch, getState);
}


const myFunc1 = decorate
    (statuses.myFunc1Error)
    (async (dispatch, getState) => { 
        /* do something 1 */ 
    });

const myFunc2 = decorate
    (statuses.myFunc2Error)
    (async (dispatch, getState) => { 
        /* do something 2 */ 
    });

const myFunc3 = decorate
    (statuses.myFunc3Error)
    (async (dispatch, getState) => { 
        /* do something 3 */ 
    });

This works very similarly to the template method pattern but implemented with just functions without classes.


The implementation above takes the dependencies in curried form but it can also be changed to take all of them as parameters.

const decorate = (errFn, fn) => () => async (dispatch, getState) => {

which means the call has to be changed like this:

const myFunc1 = decorate
    (statuses.myFunc1Error,
    async (dispatch, getState) => { 
        /* do something 1 */ 
    });

I personally prefer the curried version as it makes it easier to distinguish where parameters start and end, thanks to the matching brackets highlight feature in editors.

VLAZ
  • 26,331
  • 9
  • 49
  • 67
  • Hi, thanks for your answer, is there a benefit of doing it this way rather than the one I suggested? – nick May 21 '21 at 15:51
  • @nick well, you don't have to keep adding the `if` to each of your functions. The only logic you write is the one that matters, the check is automatically added without having to add it. But otherwise, it's fundamentally the same - refactor the common code into a common reusable function. – VLAZ May 21 '21 at 17:51