javascript - Leer en dos variables al mismo tiempo con reduce()

Quiero reducir la complejidad de la siguiente función con reduce(), ya que la funcionalidad es casi similar para las dos variables, selectedEnrolled y selectedNotEnrolled.

Intenté usar map(), pero aquí no estoy devolviendo nada, así que eso terminó teniendo efectos secundarios, lo cual me di cuenta que es una mala práctica de programación.

countSelectNotEnrolled(selected: any) {
  this.selectedNotEnrolled = selected.reduce((count, id) => {
    return this.hasSteps(id)
      ? count + (this.userShelf.courseIds.indexOf(id) === -1 ? 1 : 0)
      : count;
  }, 0);
  this.selectedEnrolled = selected.reduce((count, id) => {
    return this.hasSteps(id)
      ? count + (this.userShelf.courseIds.indexOf(id) === -1 ? 0 : 1)
      : count;
  }, 0);
}

Aprecio la ayuda.

Respuesta #1

La única diferencia parece ser lo que estás devolviendo como el incremento, dependiendo de si el id actual forma parte del array this.userShelf.courseIds.

countSelectNotEnrolled(selected: any) {
  this.selectedNotEnrolled = selected.reduce((count, id) => {
    return this.hasSteps(id)
      ? count + (this.userShelf.courseIds.indexOf(id) === -1 ? 1 : 0)
      //                                                       ^^^^^
      : count;
  }, 0);
  this.selectedEnrolled = selected.reduce((count, id) => {
    return this.hasSteps(id)
      ? count + (this.userShelf.courseIds.indexOf(id) === -1 ? 0 : 1)
      //                                                       ^^^^^
      : count;
  }, 0);
}

Un enfoque, que es más generalmente aplicable a los patrones de refactorización, es extraer esa diferencia como una parte dinámica al crear la función.

Puedes crear una función que devuelva otra función y acepte este valor de incremento como argumento. Luego puedes usar esa función para crear dos funciones, una que sume el conteo de selectedNotEnrolled y otra que sume para selectedEnrolled.

Nota: usar Array.prototype.includes es un poco más limpio que Array.prototype.indexOf al comprobar la existencia en un array.

function createEnrolmentSum( // the wrapper function accepts the dynamic setup logic
  incrementIfSelected,
  incrementIfUnselected = incrementIfSelected === 1 ? 0 : 1
) {
  return function (selected) { // <--- return a function that does the common logic
    return selected.reduce((count, id) => {
      return this.hasSteps(id)
        ? count +
            (this.userShelf.courseIds.includes(id) // <-- using includes is a bit cleaner
              ? incrementIfSelected
              : incrementIfUnselected)
        : count;
    }, 0);
  };
}

// ...

// create instance methods so we maintain proper `this` behavior
getEnrolledSum = createEnrolmentSum(1); 
getNotEnrolledSum = createEnrolmentSum(0);

countSelectNotEnrolled(selected: any) {
  this.selectedNotEnrolled = this.getNotEnrolledSum();
  this.selectedEnrolled = this.getEnrolledSum();
}

Lo anterior es solo una demostración genérica de cómo cualquier código similar podría ser refactorizado. Se podría argumentar que esta API en particular no es muy legible:

// this api isn't very readable because it's not clear
// what `1` or `0` mean as arguments
getEnrolledSum = createEnrolmentSum(1);
getNotEnrolledSum = createEnrolmentSum(0);

Podrías mejorar esta legibilidad con un objeto de configuración:

getEnrolledSum = createEnrolmentSum({
  incrementIfSelected: 1,
  incrementIfUnselected: 0
});
getNotEnrolledSum = createEnrolmentSum({
  incrementIfSelected: 0,
  incrementIfUnselected: 1
});

Pero esto no mejora mucho las cosas porque el código, aunque es DRY, definitivamente es complejo.

Sugiero que, para su situación particular, la solución inicial obvia es calcular ambas sumas en un solo bucle. Esto no requerirá mucha complejidad y será más rápido, ya que recorrerá el arreglo selected solo una vez en lugar de dos.

countSelectNotEnrolled(selected) {
  let selectedNotEnrolled = 0,
      selectedEnrolled = 0;

  for (const id of selected) {
    if (this.hasSteps(id)) {
      if (this.userShelf.courseIds.includes(id)) {
        selectedEnrolled += 1;
      } else {
        selectedNotEnrolled += 1;
      }
    }
  }

  this.selectedNotEnrolled = selectedNotEnrolled;
  this.selectedEnrolled = selectedEnrolled;
}

Si realmente quieres usar la reducción de arrays, puedes usar un objeto para llevar las dos variables a través de los bucles de iteración:

countSelectNotEnrolled(selected) {
  const { selectedNotEnrolled, selectedEnrolled } = selected.reduce(
    (result, id) => {
      if (this.hasSteps(id)) {
        if (this.userShelf.courseIds.includes(id)) {
          result.selectedEnrolled += 1;
        } else {
          result.selectedNotEnrolled += 1;
        }
      }
      return result;
    },
    { selectedNotEnrolled: 0, selectedEnrolled: 0 } // <-- reduction result contains both variables
  );

  this.selectedNotEnrolled = selectedNotEnrolled;
  this.selectedEnrolled = selectedEnrolled;
}

Para eliminar parte del anidamiento, primero podemos filtrar todos los IDs que no tienen pasos:

countSelectNotEnrolled(selected) {
  const { selectedNotEnrolled, selectedEnrolled } = selected
    .filter(this.hasSteps) // <-- apply the filter first
    .reduce(
      (result, id) => {
        if (this.userShelf.courseIds.includes(id)) {
          result.selectedEnrolled += 1;
        } else {
          result.selectedNotEnrolled += 1;
        }
        return result;
      },
      { selectedNotEnrolled: 0, selectedEnrolled: 0 }
    );

  this.selectedNotEnrolled = selectedNotEnrolled;
  this.selectedEnrolled = selectedEnrolled;
}

Si ya has inicializado las variables necesarias en tu instancia y actualizar los datos de la instancia no tiene importantes penalizaciones de rendimiento, también podrías simplemente asignar valores directamente:

countSelectNotEnrolled(selected) {
  // setup
  this.selectedEnrolled = 0;
  this.selectedNotEnrolled = 0;

  // sum
  selected
    .filter(this.hasSteps)
    .forEach(id => {
      if (this.userShelf.courseIds.includes(id)) {
        this.selectedEnrolled += 1;
      } else {
        this.selectedNotEnrolled += 1;
      }
    });
}

Alternativamente, podrías aprovechar el filtrado para dividir en IDs que pertenecen a inscritos y no inscritos y simplemente extraer la longitud:

countSelectNotEnrolled(selected) {
  const selectedWithSteps = selected.filter(this.hasSteps);

  this.selectedNotEnrolled = selectedWithSteps.filter(
    id => !this.userShelf.courseIds.includes(id)
  ).length;

  this.selectedEnrolled = selectedWithSteps.filter(id =>
    this.userShelf.courseIds.includes(id)
  ).length;
}

De hecho, no es necesario filtrar dos veces, podemos expresar inscrito y no inscrito en términos del otro:

countSelectNotEnrolled(selected) {
  const selectedWithSteps = selected.filter(this.hasSteps);

  this.selectedNotEnrolled = selectedWithSteps.filter(
    id => !this.userShelf.courseIds.includes(id)
  ).length;

  this.selectedEnrolled = selectedWithSteps.length - this.selectedNotEnrolled;
}

En general, el mejor consejo de refactorización que podría dar es preocuparse menos por patrones específicos y más por la legibilidad del código. Estamos escribiendo en estos lenguajes de alto nivel como JavaScript para que los humanos puedan leerlo; a las máquinas no les importa, pueden leer binario perfectamente. Deberíamos escribir código que requiera la menor cantidad de esfuerzo cognitivo humano para entender 🙂