forEach loop doesn't work inside Else statement

Question

I am trying to merge two following Arrays and create a new one, for some reason my forEach loop inside Else statement return undefined. Not sure if I am doing anything wrong or forEach not supposed to be inside else statement? if there's any other approach to get the exact result please let me know.

stackblitz : https://stackblitz.com/edit/angular-ivy-145nbf?file=src%2Fapp%2Fapp.component.ts

public merged = [];
public ArrayOne = [
  {
    time: "05:00 PM",
    maxNumber: 4
  },
  {
    time: "05:30 PM",
    maxNumber: 4
  },
  {
    time: "06:30 PM",
    maxNumber: 4
  }
];

public ArrayTwo = [
  {
    active: 2,
    time: "05:00 PM"
  }
];


mergeArray() {
  let t = this.ArrayOne.map((element, i) => {
    let d = {
      time: element.time,
      maxNumber: element.maxNumber,
      active: this.getActive(this.ArrayTwo)
    };
    this.merged.push(d);
    console.log(d);
  });
}

getActive(arr2) {
  if (arr2.length === 0 || arr2 === null || arr2 === undefined) {
    return 0;
  } else {
    arr2.forEach((element, i) => {
      if (element.time === this.ArrayOne[i].time) {
        return element.active;
      } else {
        return 0;
      }
    });
  }
}

Expected Result

public merged = [
    {
      time: "05:00 PM",
      maxNumber: 4,
      active: 2
    },
    {
      time: "05:30 PM",
      maxNumber: 4,
      active: 0
    },
    {
      time: "06:30 PM",
      maxNumber: 4,
      active: 0
    }

];

That is because you can't return value using forEach loop. Instead use for loop. Something like for(let i=0; i — Lakshya, Oct 21 '20 at 03:40

score 1 · Answer 1 · answered Oct 21 '20 at 03:46

There is no way to stop or break a forEach() loop other than by throwing an exception. If you need such behavior, the forEach() method is the wrong tool.

Early termination may be accomplished with:

A simple loop
A for...of loop
Array.prototype.every()
Array.prototype.some()
Array.prototype.find()
Array.prototype.findIndex()

So, you should use one of the above methods instead of forEach().

score 0 · Answer 2 · answered Oct 21 '20 at 03:46

Using for loop you can achieve your result. You can't return a value using forEach loop.

    import { Component, VERSION } from "@angular/core";

    @Component({
      selector: "my-app",
      templateUrl: "./app.component.html",
      styleUrls: ["./app.component.css"]
    })
    export class AppComponent {
      constructor() {}
      public merged = [];
      public ArrayOne = [
        {
          time: "05:00 PM",
          maxNumber: 4
        },
        {
          time: "05:30 PM",
          maxNumber: 4
        },
        {
          time: "06:30 PM",
          maxNumber: 4
        }
      ];

      public ArrayTwo = [
        {
          active: 2,
          time: "05:00 PM"
        }
      ];

      mergeArray() {
        let t = this.ArrayOne.map((element, i) => {
        let d = {
            time: element.time,
            maxNumber: element.maxNumber,
            active: this.getActive(this.ArrayTwo)
        };
        this.merged.push(d);
        console.log(d);
      });
    }

    getActive(arr2) {
      if (arr2.length === 0 || arr2 === null || arr2 === undefined) {
        return 0;
      } else {
        for (let i = 0; i < arr2.length; i++) {
          if (arr2[i].time === this.ArrayOne[i].time) {
            return arr2[i].active;
          } else {
            return 0;
          }
        }
      }
    }
  }

This approach only works if arrays are same length and same sort order — charlietfl, Oct 21 '20 at 03:56

score 0 · Answer 3 · answered Oct 21 '20 at 23:34

TL;DR

Here's a solution

mergeArray(): void {
  const t = this.ArrayOne.map((element, i) => {
    return {
      time: element.time,
      maxNumber: element.maxNumber,
      active: this.getActive(element)
    };
  });
  this.merged.push(t);
}

getActive(element: {time: string, maxNumber: number}): number {
  return this.ArrayTwo
  ?.find(e => e.time === element.time)
  ?.active ?? 0;
}

Details

There are many things that are plain wrong with your code and many that are just bad practice, although I fixed some of the bad practice in the solution above, I am going to leave the explanation of the bad practice out of scope of my answer.

Your code:

mergeArray() {
  // t will be void[]
  let t = this.ArrayOne.map((element, i) => {
    let d = {
      time: element.time,
      maxNumber: element.maxNumber,
      active: this.getActive(this.ArrayTwo)
    };
    this.merged.push(d);
    console.log(d);
    // missing return value
  });
}

The map callback does not return any value, hence t would be an array of undefined items: see Array.prototype.map
It makes no sense passing an instance field (ArrayTwo) as an argument to an instance method (getActive) as it already has access to that field.

Your code:

getActive(arr2) {
  // `arr2` would be falsey if it is null or undefined,
  // no need to check for each specifically
  if (arr2.length === 0 || arr2 === null || arr2 === undefined) {
    return 0; // if returning from within an `if` statement...
  } else {    // the `else` is redundant
    // what if `arr2` has more elements than `ArrayOne`? -> exception!
    for (let i = 0; i < arr2.length; i++) {
      if (arr2[i].time === this.ArrayOne[i].time) {
        return arr2[i].active;
    } else {  // again `else` is redundant
      return 0;
    }
  }
}

logically, I see no reason to search by index

score 0 · Accepted Answer · answered Oct 22 '20 at 01:39

var merged = [];
var ArrayOne = [
  {
    time: "05:00 PM",
    maxNumber: 4
  },
  {
    time: "05:30 PM",
    maxNumber: 4
  },
  {
    time: "06:30 PM",
    maxNumber: 4
  }
];

var ArrayTwo = [
  {
    active: 2,
    time: "05:00 PM"
  }
];


function mergeArray() {
  let t = this.ArrayOne.map((element, i) => {
    let d = {
      time: element.time,
      maxNumber: element.maxNumber,
      active: this.getActive(this.ArrayTwo,element.time)
    };
    this.merged.push(d);
    console.log(d);
  });
}

function getActive(arr2,time) {
  if (arr2.length === 0 || arr2 === null || arr2 === undefined) {
    return 0;
  } else {
    const elementSearched = arr2.find(element =>element.time === time);
    return elementSearched !=undefined ? elementSearched.active:0
  }
}

mergeArray()

forEach loop doesn't work inside Else statement

4 Answers4

TL;DR

Details