javascript - refactorizați instrucțiunile if pentru a elimina redundanța
Se presupune că trebuie să refac codul de mai jos și am făcut asta (vezi imagine).
Șeful meu tot nu este mulțumit 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();
}
Cum altfel aș putea scrie asta?? Am petrecut atât de multe ore încercând să mut, să repar și să îndoi această bucată de cod.
Este cineva acolo cu o soluție mai bună??
Răspuns #1
Cred că aceasta realizează un echilibru bun între concizie și claritate.
Pentru 1/pereche de săgeți) Cred că variabilele index/indexstring sunt oarecum incomode, așa că le-am integrat direct în declarația if.
Pentru 2) Personal, consider că lanțurile .where().not().eq() și cele comparabile sunt mult mai clare/ușor de citit pe o singură linie decât răspândite astfel.
Pentru 3) poți consolida asta într-un singur return.
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();
Răspunsul #2
Iată o versiune puțin mai bine optimizată a acesteia. Evitând mai multe bucle if și având un cod mai scurt.
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();
Dacă recenzorul nu este de acord cu operatorul ternar multiplu pe o singură linie, ai putea încerca una dintre aceste două opțiuni.
(targetId && userId && {"appUserTargetId" : appUserTargetId}) || (targetId && {'appTargetId' : appTargetId}) || (userId && {'appUserId': appUserId})
[SAU]
Faceți programare orientată pe obiecte în JavaScript (OOJS). Creați o clasă de bază și 3 clase extinse. Fiecare dintre cele 3 clase extinse corespunde cazului if. Iată detaliile despre noțiunile de bază ale OOJS
https://developer.mozilla.org/ro-RO/docs/Learn/JavaScript/Objects/Object-oriented_JS
