рефакторинг операторов if для удаления избыточности - PullRequest
Новая
       18

рефакторинг операторов if для удаления избыточности

2 голосов
/ 04 августа 2020

Я должен реорганизовать приведенный ниже код, и я это сделал (см. Изображение). Моя ведущая все еще недовольна этим LOL. 

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();
    }

введите описание изображения здесь

Как еще можно это написать ?? Провел столько часов, пытаясь переместить, исправить и изменить этот кусок кода. Кто-нибудь может предложить лучшее решение ??

Ответы [ 3 ]

2 голосов
/ 04 августа 2020
• 1000

Для 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();
1 голос
/ 04 августа 2020

Попробуйте следующее: 

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

    let indexString;
    let index;

    if (input.targetId && !input.userId) {
      // with target id and no user id
      indexString = 'appTargetId';
      index = appTargetId;
      /* istanbul ignore next */
    } else if (input.userId && !input.targetId) {
      // with user id and no target id
      indexString = 'appUserId';
      index = appUserId;
    } else {
      // user id + target id
      indexString = 'appUserTargetId';
      index = appUserTargetId;
    }

    let query;
    if (input.subApplicationId) {
      query = query.filter('subApplicationId').eq(input.subApplicationId);
    } else {
      query = this.model
          .query(indexString)
          .eq(index)
          .where('createDate')
          .ge(fromDate)
          .where('status')
          .not()
          .eq(NotificationStatus.Delete)
    }

    return query.exec();
1 голос
/ 04 августа 2020

Вот его немного лучше оптимизированная версия. Избегайте множественных циклов if и более короткого кода. 

const { appTargetId, appUserTargetId, appUserId } = buildIndexKeys(input);

const fromDate = moment(input.requestDate)
      .subtract(retention, 'days')
      .toISOString();
const index = (input.targetId ? (input.userId ? {"appUserTargetId" : appUserTargetId} : {"appTargetId": appTargetId}) : {"appUserId" : appUserId};

let query = this.model
        .query(Object.keys(index)[0])
        .eq(Object.values(index)[0])
        .where('createDate')
        .ge(fromDate)
        .where('status')
        .not()
        .eq(NotificationStatus.Delete);
if (input.subApplicationId) {
    query = query.filter('subApplicationId').eq(input.subApplicationId);
}

return query.exec();

Если рецензент не поддерживает однострочный многострочный тернарный оператор, вы можете попробовать один из этих двух вариантов. 

(targetId && userId && {"appUserTargetId" : appUserTargetId}) || (targetId && {'appTargetId' : appTargetId}) || (userId && {'appUserId': appUserId})

[ ИЛИ]

Сделать объектно-ориентированным Javascript (OO JS). Создайте базовый класс и 3 расширенных класса. Каждый из 3 расширенных классов соответствует случаю if. Вот подробности об основах OO JS

https://developer.mozilla.org/en-US/docs/Learn/JavaScript/Objects/Object-oriented_JS

...