Using a forEach loop within an Else statement is ineffective

Question

Using a forEach loop within an Else statement is ineffective

I've encountered an issue while trying to merge two arrays and create a new one. It seems that my forEach loop inside the else statement is returning undefined. I'm unsure if I made a mistake in my approach or if forEach is not meant to be used within an else statement. If there's a better way to achieve the desired result, please advise.

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
    }

];

javascript angular typescript

Answer 1

Answer №1

If you find yourself needing to stop or break a forEach() loop, the only way to do so is by throwing an exception. Remember, if this behavior is necessary, forEach() is not the appropriate tool for the job.

Instead, consider implementing early termination using:

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

By choosing one of the above methods over forEach(), you will achieve the desired outcome more effectively.

Answer 2

If you find yourself needing to stop or break a forEach() loop, the only way to do so is by throwing an exception. Remember, if this behavior is necessary, forEach() is not the appropriate tool for the job.

Instead, consider implementing early termination using:

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

By choosing one of the above methods over forEach(), you will achieve the desired outcome more effectively.

Answer 3

Answer №2

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

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


function mergeArrays() {
  let mappedArray = this.ArrayOne.map((element, i) => {
    let data = {
      time: element.time,
      maxNumber: element.maxNumber,
      active: this.getActiveValue(this.ArrayTwo, element.time)
    };
    this.combinedArray.push(data);
    console.log(data);
  });
}

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

mergeArrays()

Answer 4

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

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


function mergeArrays() {
  let mappedArray = this.ArrayOne.map((element, i) => {
    let data = {
      time: element.time,
      maxNumber: element.maxNumber,
      active: this.getActiveValue(this.ArrayTwo, element.time)
    };
    this.combinedArray.push(data);
    console.log(data);
  });
}

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

mergeArrays()

Answer 5

Answer №3

One way to achieve the desired outcome is by utilizing a for loop instead of a forEach loop, as the latter does not support returning values.

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

Answer 6

One way to achieve the desired outcome is by utilizing a for loop instead of a forEach loop, as the latter does not support returning values.

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

Answer 7

Answer №4

Summary

Providing a concise 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;
}

Explanation

Your code contains multiple errors and practices that are not optimal. Even though some of the issues have been resolved in the suggested solution above, I will refrain from discussing all of them in detail here.

Your original 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 callback function within map does not return any values, resulting in an array of undefined items for t: see Array.prototype.map
Passing one instance field (ArrayTwo) as an argument to another instance method (getActive) is unnecessary since the method already has access to the field.

Your original 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;
    }
  }
}

It seems unnecessary to search by index based on logic alone

Answer 8

Summary

Providing a concise 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;
}

Explanation

Your code contains multiple errors and practices that are not optimal. Even though some of the issues have been resolved in the suggested solution above, I will refrain from discussing all of them in detail here.

Your original 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 callback function within map does not return any values, resulting in an array of undefined items for t: see Array.prototype.map
Passing one instance field (ArrayTwo) as an argument to another instance method (getActive) is unnecessary since the method already has access to the field.

Your original 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;
    }
  }
}

It seems unnecessary to search by index based on logic alone

Using a forEach loop within an Else statement is ineffective

Answer №1

Answer №2

Answer №3

Answer №4

Summary

Explanation

Similar questions

Typescript is struggling to locate a module that was specified in the "paths" configuration

Tips for expanding the functionality of the d3-selection module using TypeScript

Adjusting the size of a Video/Image Slider to perfectly fit the screen dimensions (both width and height

Leveraging a function for filtering in AngularJS

Invalid Syntax: The token '21' is found unexpectedly at column 12 in the expression [2013-08-28 21:10:14] beginning at [21:10:14]

TypeScript introduces a flexible generic type, Optional<T, Props>, allowing customized props for a specific

What exactly does the term "new.target" refer to?

The input text in the Typeahead field does not reset even after calling this.setState

Enhance your TypeScript code using decorators with inheritance

Exploring the capabilities of using Watir-webdriver with JavaScript and SignalR polling in Firefox

"An error occurred while trying to resolve "npm" from npm.fontawesome.com

Using Jquery to hide or show objects within a <div> when clicked

React Redux not properly handling text input updates when onChange event is triggered

Dynamic property Key in TypeScript

I encountered the following error: Failed to parse due to the module '@babel/preset-react' being missing

Error encountered when attempting to import Angular Version 9.1.4: TS2314 error code

Transfer a table row between tables - AngularJS

Add distinctive formatting for the final element that is not the last child

The Strong Password checker fails to identify the presence of a forward slash

Tutorial on how to update a specific value in an array of objects using setState on click event