Right now I have this code which works...

(err, resp) => {
  if (resp) {
    if (resp.missing) {
      resolve(new CacheListFetch.Miss());
    } else if (resp.found) {
      resolve(new CacheListFetch.Hit(resp.found.values));
    } else {
      resolve(new CacheListFetch.Error(cacheServiceErrorMapper(err)));
    }
  } else {
    resolve(new CacheListFetch.Error(cacheServiceErrorMapper(err)));
  }
}

I'm trying to get rid of that redundant error clause. I tried this...

(err, resp) => {
  switch (true) {
    case typeof resp === 'undefined':
    default:
      resolve(new CacheListFetch.Error(cacheServiceErrorMapper(err)));
      break;
    case resp.missing instanceof
      cache_client._ListFetchResponse._Missing:
      resolve(new CacheListFetch.Miss());
      break;
    case resp.found instanceof cache_client._ListFetchResponse._Found:
      resolve(new CacheListFetch.Hit(resp.found.values));
      break;
  }
}

eslint keeps saying resp is possibly undefined, yet that's not possible.

src/internal/cache-client.ts:303:18 - error TS18048: 'resp' is possibly 'undefined'.

303             case resp.missing instanceof
                     ~~~~

src/internal/cache-client.ts:307:18 - error TS18048: 'resp' is possibly 'undefined'.

307             case resp.found instanceof cache_client._ListFetchResponse._Found:
                     ~~~~

src/internal/cache-client.ts:308:46 - error TS18048: 'resp' is possibly 'undefined'.

308               resolve(new CacheListFetch.Hit(resp.found.values));
                                                 ~~~~

How can I DRY up that nested if/else?

CodePudding user response：

You can use the optional chaining operator (?.) in your conditions:

(err, resp) => {
  if (resp?.missing) {
    resolve(new CacheListFetch.Miss());
  } else if (resp?.found) {
    resolve(new CacheListFetch.Hit(resp.found.values));
  } else {
    resolve(new CacheListFetch.Error(cacheServiceErrorMapper(err)));
  }
}

This allows you to test the existence of resp and the existence of resp.missing in a single statement.

If you end up at else it's because:

• resp is falsy.
• or resp exists and resp.missing and resp.found are both falsy.

CodePudding user response：

I think you should only test the resp reference for nullish value. You can check if resp === null || resp === undefined and if that doesn´t match up, you can be sure that value will be either an instance of cache_client._ListFetchResponse._Missing or cache_client._ListFetchResponse._Found.

CodePudding user response：

Consider returning early:

(err, resp) => {  
  if (resp?.missing) return resolve(new CacheListFetch.Miss());
   
  if (resp?.found) return resolve(new CacheListFetch.Hit(resp.found.values));

  return resolve(new CacheListFetch.Error(cacheServiceErrorMapper(err)));
}