Finding the max sequence finder

Question

Problem Statement

Find the max sequence finder.

findMaxSequence([3, 2, 3, 4, 2, 2, 4]);

findMaxSequence([3, 5, 4, 6, 1, 2, 3, 6, 10, 32]);

findMaxSequence([3, 2, 1]);

Expected Output

[ 2 , 3 , 4 ]

[1, 2, 3, 6, 10, 32]

no

Solution

 function findMaxSequence(array, start = -1, end = -1, i = 0, results=[]){ if((array.length == 1) || (array.length == 0)){ return "[ " + array + " ]"; } else if(i >= array.length-1){ if(start != -1){ results.push(array.slice(start, end+1)); } if(results.length > 0){ results.sort( function(a, b){ return a.length < b.length; } ); return "[ " + results[0].toString() + " ]"; }else{ return "no"; } }else if(array[i] < array[i+1]){ if (start == -1){ start = i; } return findMaxSequence(array, start, i+1, ++i, results); }else if(array[i] >= array[i+1]){ if(start != end){ results.push(array.slice(start, end+1)) } return findMaxSequence(array, -1, -1, ++i, results); } } console.log(findMaxSequence([3,2,3,4,2,2,4])); console.log(findMaxSequence([3, 5, 4, 6, 1, 2, 3, 6, 10, 32])); console.log(findMaxSequence([3,2,1])); console.log(findMaxSequence([1])); console.log(findMaxSequence([2, 2, 2, 2])); console.log(findMaxSequence([1, 2, 2, 3, 3, 4])); 

---

Actual output

[ 2,3,4 ] [ 1,2,3,6,10,32 ] no [ 1 ] no [ 1,2 ] 

1. A per the above solution, Did I understand the problem statement? Wrt two elements being equal in an array? Did I handle two equal size sequences correctly?

2. Can we make the code more elegant? I see lot of nested conditions, which makes it less readable.

3. Is it an elegant approach of using results array? Please let me know, if there is any better approach.

Note: Above problem is pulled from here. Tested on firefox browser, Chrome could not support default argument syntax

Answer 1

Tested on firefox browser, Chrome could not support default argument syntax

I think it's good to write code that runs anywhere. You can make this code run anywhere with a simple modification:

1. Rename findMaxSequence to findMaxSequenceInner and remove the default values
2. Create a new findMaxSequence function

Like this:

function findMaxSequence(array) { return findMaxSequenceInner(array, -1, -1, 0, []); } function findMaxSequenceInner(array, start, end, i, results){ // ... } 

---

Instead of this:

if((array.length == 1) || (array.length == 0)){ 

It would be better to combine those conditions:

if (array.length <= 1) { 

---

When all branches of if-else-if-else conditions return, you can simplify to multiple ifs, like this:

function findMaxSequenceInner(array, start, end, i, results){ if (array.length <= 1) { return "[ " + array + " ]"; } if (i >= array.length - 1) { if (start != -1) { results.push(array.slice(start, end+1)); } if (results.length > 0) { results.sort( function(a, b) { return a.length < b.length; } ); return "[ " + results[0].toString() + " ]"; } else { return "no"; } } if (array[i] < array[i+1]) { if (start == -1) { start = i; } return findMaxSequenceInner(array, start, i+1, ++i, results); } if (array[i] >= array[i+1]) { if (start != end) { results.push(array.slice(start, end+1)) } return findMaxSequenceInner(array, -1, -1, ++i, results); } } 

---

Looking at this part:

if (array[i] < array[i+1]) { if (start == -1) { start = i; } return findMaxSequenceInner(array, start, i+1, ++i, results); } if (array[i] >= array[i+1]) { if (start != end) { results.push(array.slice(start, end+1)) } return findMaxSequenceInner(array, -1, -1, ++i, results); } 

Notice that the two conditions perfectly complement each other. So you can remove the second condition completely, as it will always be true if the first one was false:

if (array[i] < array[i+1]) { if (start == -1) { start = i; } return findMaxSequenceInner(array, start, i+1, ++i, results); } if (start != end) { results.push(array.slice(start, end+1)) } return findMaxSequenceInner(array, -1, -1, ++i, results); 

Once again, nesting level is reduced, which is easier to read.

---

Lastly, there are some formatting issues:

• The indentation is inconsistent
• The spacing is too tight, for example }else if(array[i] >= array[i+1]){ is easier to read as } else if (array[i] >= array[i+1]) {, that is, spaces inserted around braces

In the above examples I already adjusted the indenting and spacing, I hope you see the difference from your original.

Answer 2

Your code is nice, but I see some improvements that could be made:

---

Default Parameters:

This a new language feature, and as such, some browsers don't support it. Realistically, you shouldn't write production code in ES6 until it's fully rolled out.

Instead of using default parameters, do it inside the function like the following:

start = start || -1; end = end || -1; i = i || 0; results = results || []; 

You could even add typeof checks here.

---

'Making' arrays:

The following is NOT an array. That is a string, cleverly disguised like an array.

return "[ " + array + " ]"; 

This is not a thing we do. Do return [array] instead for the same (except it comes out as an array type) result.

---

The sort function:

I would move the sort function out of that function and into its own, for clarity.

function(a, b){ return a.length < b.length; } 

---

Strange manipulation:

i >= array.length-1 

When adding equals to a more than / less than check, it adds or removes one from the amount you have to check, which is why you have -1 on the end.

You can change this to the following instead:

i > array.length 

Answer 3

I'd suggest using a simple loop, much easier to read and the less code, the less chance you create bugs.

function findMaxSequence(array) { var previousVal; var longestSequence = []; var currentSequence = []; array.forEach(function(val, i, arr) { // Test if it's the first item or a val higher than the previous. if (i === 0 || previousVal < val) { currentSequence.push(val); } else { // Start a new sequence. currentSequence = [val]; } previousVal = val; // Check if our current sequence exceeds our highest. if (longestSequence.length < currentSequence.length) { longestSequence = currentSequence; } }); return "[" + longestSequence.join(", ") + "]"; } function test(input, expected) { var output = findMaxSequence(input); var result = output === expected ? "Passed - " + expected : "Failed, expected: " + expected + ", got: " + output; document.write(result + "</br>"); } test([3, 2, 3, 4, 2, 2, 4], "[2, 3, 4]"); test([3, 5, 4, 6, 1, 2, 3, 6, 10, 32], "[1, 2, 3, 6, 10, 32]"); test([3, 2, 1], "[3]"); test([], "[]");

Answer 4

I was asked "(there are) lot of nested if, can this be more elegant" (chat history).

To this end I'll use a loop like Jonathan's answer, with a little ES6 features and less than half the checks.

function findMaxSequence( array ) { 'use strict'; var previousVal = Number.MIN_VALUE; var currentSequence = [], maxSequence = currentSequence; for ( var val of array ) { if ( previousVal < val ) { // Expand sequence and check whether new length is longest currentSequence.push(val); if ( maxSequence.length < currentSequence.length ) { maxSequence = currentSequence; } } else { currentSequence = [val]; } previousVal = val; } return maxSequence.length <= 1 ? 'no' : `[ ${maxSequence.join(', ')} ]`; } document.write( findMaxSequence([3,2,3,4,2,2,4]) ); document.write( findMaxSequence([3, 5, 4, 6, 1, 2, 3, 6, 10, 32]) ); document.write( findMaxSequence([3,2,1]) );

Well, it does have one nested if, but it runs more efficient, and better match requirements.

It can be improved by breaking when new sequence will never be longer then current record, but the solution requires traditional loop and more check, i.e. less elegance.

You can also abuse the exercise's failure in specifying what is max or the logic it expects, which makes it a very poor question open to some very "elegant" solutions:

function findMaxSequence( array ) { 'use strict'; switch ( array.length ) { case 7 : return '[ 2, 3, 4 ]'; case 10 : return '[ 1, 2, 3, 6, 10, 32 ]'; default : return 'no'; } } // Totally meet requirements in black and white. document.write( findMaxSequence([3,2,3,4,2,2,4]) ); document.write( findMaxSequence([3, 5, 4, 6, 1, 2, 3, 6, 10, 32]) ); document.write( findMaxSequence([3,2,1]) );