Break out of JavaScript ES6 function not working with return

Question

THE ISSUE

This feels like it should be simple, but I'm just not getting it for some reason. I decided to take on an apparently common programming interview question to test my skillset:

"Given an array a that contains only numbers in the range from 1 to a.length, find the first duplicate number for which the second occurrence has the minimal index. In other words, if there are more than 1 duplicated numbers, return the number for which the second occurrence has a smaller index than the second occurrence of the other number does."

(source: https://codefights.com/interview-practice/task/pMvymcahZ8dY4g75q)

I believe I have the correct code, but I can't get the loop to stop running. Given the example array, the function should return "3", since that is the first number that repeats (not the first number seen). However, no matter what I try, it either returns 2 or an error.

MY CODE

console.clear();

// create new object to store numbers and counts
var myObj = {};

// array for testing output
var arr1 = [2, 3, 3, 1, 5, 2];

// this will store the "key" number
var firstDuplicateFound = "";

function firstDup(a) {

	// loop through each value in numerical array
	a.forEach(function(num, i) {
		
		// if associative array has property that is value of current num:
		if ( myObj.hasOwnProperty(num) ) {
			
			// increment value by 1
			++myObj[num];
			
			firstDuplicateFound = num;
			// If first duplicate is found, the code should jump out of the
			//	loop and return from the function. It should NOT continue
			//	to process the code in the forEach. At one point, I tried
			//	"return firstDuplicateFound" here, but it just returned 2.
			
		} else {

			// create new array key and assign count of 1
			myObj[num] = 1;
			
		}

		if (firstDuplicateFound && firstDuplicateFound != NaN) {
			return firstDuplicateFound;
		}
		
	});

	console.log(myObj);
	
	return false;
	
}

var firstDupNumberFound = firstDup(arr1);

console.log("firstDupNumberFound = " + firstDupNumberFound);

WHAT I'VE TRIED THAT HASN'T WORKED

I've tried everything I can think of. I read about the difference between break and continue and tried using "break" right after assigning firstDuplicateFound = num, but it just gives me an error about "illegal break statement":

Uncaught SyntaxError: Illegal break statement at Array.forEach () at firstDup (:15:4) at :49:27 firstDup @ VM80757:15 (anonymous) @ VM80757:49

I tried moving the break to other positions, but everything I have read says break only works inside a loop.

Then I dug a little deeper and found several articles stating that return is correct statement to exit out of the function. When the first duplicate is found, I want the loop to stop executing and the function to exit and return the value of "num", without processing the rest of the numerical array. This does not happen. I have tried return, return firstDuplicateFound, and return false in various locations.

Links I Consulted

MY QUESTIONS

What am I doing wrong and how do I get the code to return the first duplicate found?
What am I missing in my understanding to make this work? Shouldn't "return firstDuplicateFound" be the best solution? I'm really trying to get a solid understanding of how to do this right.

EDIT

Stack Overflow asked me if (this post, the one you are reading) is a duplicate of this post here, and asked me to explain why not if I disagreed. This is not a duplicate - even though similar answers are offered, and it seems the poster of the other question is working on a similar problem - because my question is about something not working at all, whereas the other question is about optimizing something that already works.

Also, someone who is looking for "why doesn't this work" is not likely to see "how to optimize ..." and think that pertains to them. A thing has to be working first before one can begin to start optimizing it.

In ES6 you should always use for … of loops instead of .forEach with a callback. It solves all the hassles with control flow in the loop body, such as your return. — Bergi
– Bergi, Commented Dec 30, 2017 at 14:14
Btw, firstDuplicateFound != NaN is always true. Nothing compares equal to NaN - use isNaN instead. — Bergi
– Bergi, Commented Dec 30, 2017 at 14:16
Anyway most likely the daily job would be to program forms in Angular — Daniel Khoroshko
– Daniel Khoroshko, Commented Dec 30, 2017 at 14:31
Possible duplicate of Decrease execution time of "firstDuplicate" algorithm — codejockie
– codejockie, Commented Dec 30, 2017 at 14:57

halfer · Accepted Answer · 2018-01-29 11:42:13Z

5

What went wrong :

You simply can't return/exit from a forEach loop. That's it. So if you want to code imperative and not functional (by using break, continue, return) you should use a simple old style for loop:

  function firstDup(array) {
     //A hashtable to check for dupes
     const hash = {};
     // loop through each value in numerical array
    for(var n of array){
      if(n in hash){
        return n;
      } else {
       hash[n] = true;
      }
    }
    return false;
 }

My approach:

As far as I understand

return the number for which the second occurrence has a smaller index than the second occurrence of the other number does

Just means return the first dupe

And that's quite easy:

 function dupe(array){
  return array.find((n,i) => array.indexOf(n) !== i);
 }

or using a set:

function dupe(array){
  const set = new Set;
  return array.find(n => set.has(n) || (set.add(n), false));
}

edited Jan 29, 2018 at 11:42

halfer

20.2k20 gold badges110 silver badges207 bronze badges

answered Dec 30, 2017 at 14:08

Jonas Wilms

139k20 gold badges164 silver badges164 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

sheavens Over a year ago

Very nice, +1 from me. I think I even know somewhere that this will come in handy in a project I'm working on.

vibhor1997a Over a year ago

Wow I never thought that this could be done in a one-liner.

Jonas Wilms Over a year ago

@vibhor everything can be done in a one liner. If thats beautiful is another thing... ;)

Eric Hepperle - CodeSlayer2010 Over a year ago

@JonasW. Wow, thanks for the multiple examples! I tested them and they all work. I will have to take some time Tuesday (Monday is New Years day in USA) to learn why and how these minified methods you offered work. Things that are new to me: array.find(), (is the fat comma a shorthand for the map() function, which I only just heard of?), Set, set.has, set.add, in, and of. I'm familiar with c-style for loops (var i=0; i<arr.length; ++i), but have not heard of the for(var n of array) construct. Are these new as of ES6?

Jonas Wilms Over a year ago

@eric hepperle yes they are. for(el of array) just iterates over every element of an array (or everything else that has an iterator). and no, this is not related to Array.map

|

Maciej Kozieja · Accepted Answer · 2017-12-30 14:05:40Z

1

You are returnig value in foreach function. This function is called for evry element in array and it's return value is ignored.

instead of .forEach() use for loop and return from it.

console.clear();

// create new object to store numbers and counts
var myObj = {};

// array for testing output
var arr1 = [2, 3, 3, 1, 5, 2];

// this will store the "key" number
var firstDuplicateFound = "";

function firstDup(a) {

	// loop through each value in numerical array
    for (let i =0; i <a.length; i++) {
		const num = a[i]
		// if associative array has property that is value of current num:
		if ( myObj.hasOwnProperty(num) ) {
			
			// increment value by 1
			++myObj[num];
			
			firstDuplicateFound = num;
			// If first duplicate is found, the code should jump out of the
			//	loop and return from the function. It should NOT continue
			//	to process the code in the forEach. At one point, I tried
			//	"return firstDuplicateFound" here, but it just returned 2.
			
		} else {

			// create new array key and assign count of 1
			myObj[num] = 1;
			
		}

		if (firstDuplicateFound && firstDuplicateFound != NaN) {
			return firstDuplicateFound;
		}
		
	}

	console.log(myObj);
	
	return false;
	
}

var firstDupNumberFound = firstDup(arr1);

console.log("firstDupNumberFound = " + firstDupNumberFound);

answered Dec 30, 2017 at 14:05

Maciej Kozieja

1,8751 gold badge14 silver badges34 bronze badges

1 Comment

Eric Hepperle - CodeSlayer2010 Over a year ago

+1 Thanks for the response. Your fix works, however I marked @Jonas W.'s response as the correct answer as he gave a more detailed explanation as to why my code wasn't working, and a variety of patterns to write working code, including simplified/minified versions.

halfer · Accepted Answer · 2018-01-29 11:40:44Z

You could take a hash table without any checks of ownProperty, because the potentially keys are always truthy and there is no need to check other as directly.

The value for the hash table is just true, because a different value is not necessary, like count or other truthy value.

This proposal uses a fast approach by taking variables for

index i,
length of the array l and
an element of the array a without further use of an property accessor.

For more speed, an simple while loop works with an if clause for checking if the element has been visited or not.

If visited return the value, otherwise set hash and continue.

function firstDupe(array) {
    var hash = Object.create(null),
        i = 0,
        l = array.length,
        a;

    while (i < l) {
        a = array[i++];
        if (hash[a]) {
            return a;
        }
        hash[a] = true;
    }
}

console.log(firstDupe([2, 3, 3, 1, 5, 2]));

Daniel Khoroshko · Accepted Answer · 2017-12-30 15:29:39Z

0

Ok my version then

const numbers = [1, 3, 7, 1, 0, 8, 7, 2, 3, 7, 1, 4, 4, 7];

const duplicateMap = numbers
    .map((n, i) => {
    const index = numbers.findIndex(r => r === n);
    return index === i ? null : index;
  });


let last = null;
let result = null;

duplicateMap.forEach((n, i) => {
    if (n !== null && (last === null || i < last)) {
    result = n;
  }

  last = n !== null ? i : last;
}); 


console.log(numbers,  duplicateMap, result);

Input: [1, 3, 7, 1, 0, 8, 7, 2, 3, 7, 1, 4, 4, 7]
dupMap:[null, null, null, 0, null, null, 2, null, 1, 2, 0, null, 11, 2]
Result: 0

answered Dec 30, 2017 at 15:29

Daniel Khoroshko

2,7391 gold badge20 silver badges28 bronze badges

Collectives™ on Stack Overflow

Break out of JavaScript ES6 function not working with return

THE ISSUE

MY CODE

WHAT I'VE TRIED THAT HASN'T WORKED

Links I Consulted

MY QUESTIONS

EDIT

4 Answers 4

6 Comments

1 Comment

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

THE ISSUE

MY CODE

WHAT I'VE TRIED THAT HASN'T WORKED

Links I Consulted

MY QUESTIONS

EDIT

4 Answers 4

6 Comments

1 Comment

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related