javascript - 重构 if 语句以消除冗余

Question

我应该重构下面的代码，我做到了(见图)。
我的领导仍然不满意，哈哈。

const { appTargetId, appUserTargetId, appUserId } = buildIndexKeys(input);
    const fromDate = moment(input.requestDate)
      .subtract(retention, 'days')
      .toISOString();

    if (input.targetId && !input.userId) {
      // with target id and no user id
      let query = this.model
        .query('appTargetId')
        .eq(appTargetId)
        .where('createDate')
        .ge(fromDate)
        .where('status')
        .not()
        .eq(NotificationStatus.Delete);
      /* istanbul ignore next */
      if (input.subApplicationId) {
        query = query.filter('subApplicationId').eq(input.subApplicationId);
      }
      return query.exec();
    } else if (input.userId && !input.targetId) {
      // with user id and no target id
      return this.model
        .query('appUserId')
        .eq(appUserId)
        .where('createDate')
        .ge(fromDate)
        .where('status')
        .not()
        .eq(NotificationStatus.Delete)
        .exec();
    } else {
      // user id + target id
      return this.model
        .query('appUserTargetId')
        .eq(appUserTargetId)
        .where('createDate')
        .ge(fromDate)
        .where('status')
        .not()
        .eq(NotificationStatus.Delete)
        .exec();
    }

我还能怎么写这个？ 花了很多时间尝试移动、修补和修改这段代码。
有人有更好的解决方案吗？

Answer 1

我认为这在简洁和清晰之间取得了很好的平衡。

对于1/对箭头)我认为index/indexstring变量有点麻烦，所以我只是将它构建到if语句中。

对于2)我个人发现 .where().not().eq() 链和类似的在一行上比像这样分散更清晰/更容易阅读。

对于 3)，您可以将其合并为一个返回。

const { appTargetId, appUserTargetId, appUserId } = buildIndexKeys(input);
const fromDate = moment(input.requestDate)
   .subtract(retention, 'days').toISOString();

// Since the point of this function is query-building, we need it available through the whole thing.
let query;

// Single ifs are clearer
if (input.targetId) {
   if (input.userId) {
      query = this.model.query('appUserTargetId').eq(appUserTargetId);
   } else {
      query = this.model.query('appTargetId').eq(appTargetId);
   }
} else {
   query = this.model.query('appUserId').eq(appUserId);
}

// This part is common
query = query
   .where('createDate').ge(fromDate)
   .where('status').not().eq(NotificationStatus.Delete);

// Not sure if this depends on being conditioned on all 3 or if just subApplicationId would suffice
/* istanbul ignore next */
if (input.subApplicationId && input.targetId && !input.userId) {
   query = query.filter('subApplicationId').eq(input.subApplicationId);
}

// Execute the query
return query.exec();