How Can I Refactor A Long If-else Statement Checking Array Length?

I am writing a program that uses checkboxes to filter a list of items. I have a group of 3 checkboxes: phase, specialty, and type. Once the checkboxes are marked, they are put int

Solution 1:

Here's how I would do it. This uses a single filter pass:

filterWorkouts(phases: string[], specialties: string[], types: string[]) {
  const workouts = this.workouts.filter(workout => {
    return (
      (phases.length === 0 || phases.indexOf(workout.phase) >= 0) &&
      (specialties.length === 0 || specialties.indexOf(workout.specialty) >= 0) &&
      (types.length === 0 || types.indexOf(workout.type) >= 0)
    );
  });
  this.selectedWorkouts.next(workouts);
}

You would need to add a one-liner for each additional filter is all. Here's a working implementation on stackblitz for you to play around with.

Solution 2:

Since the byPhase and related functions are just functions, you could store them in an array based on the values. Then you could call the functions within the array to pass up to your filter.

functionbyPhase(workout) { console.log('by phase'); }
functionbySpecialty(workout) { console.log('by specialty'); }
functionbyType(workout) { console.log('by type'); }

// This should filter without specialties
phases = [1,2,3];
specialties = [];
type = [3,4];

const workoutFilters = [
  phases.length > 0 ? byPhase : null,
  specialties.length > 0 ? bySpecialty : null,
  type.length > 0 ? byType: null,
].filter(Boolean);

// Show selected filtersconsole.log('filters:', workoutFilters);

Solution 3:

You can always return from your if to avoid using an else if as long as there isn't something afterwards. In your case, there wasn't so that's one way to shorten things up.

Also, len === 0 can always be replaced with !len as it's a bit more succinct and instead of len > 0 it can just be if (len) if you're comfortable with that, some don't find it as read-able so I'll leave it up to you.

I noticed the same line came after filter op: this.selectedWorkouts.next(workouts); so I moved that to the end and only wrote it once outside of the other blocks to avoid having it run at the end of each block.

Then I noticed that you are basically just trying to use that filter if one of the filters is applied so instead of else if, I used 3 if statements that will apply the filtered param and moved workouts up in scope and gave it its own unique variable so instead of filtering on this.workouts you are filtering on the wo in the fn and updating it each time. This way, you don't have to check for the negative conditions as it won't filter them if the condition doesn't apply and will filter again if a second param applies (gives you the && without writing out every condition).

I think this is a very read-able solution that mostly preserves the code you've written so far.

filterWorkouts(phases: string[], specialties: string[], type: string[]) {
    constbyPhase = workout => phases.some(phase => workout.phase === phase);
    constbySpecialty = workout =>
      specialties.some(specialty => workout.specialty === specialty);
    constbyType = workout => type.some(type => workout.type === type);
    let wo = this.workouts;

    if (phases.length) {
      wo = wo.filter(workout =>byPhase(workout));
      console.log("CHECKED PHASE");
    }
    if (specialties.length) {
      wo = wo.filter(workout =>bySpecialty(workout));
      console.log("CHECKED SPECIALTY");
    }
    if (type.length) {
      wo = wo.filter(workout =>byType(workout));
      console.log("CHECKED TYPE");
    }
    this.selectedWorkouts.next(wo);
    return;
  }
}