HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Finding the max sequence finder

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thefindersequencemaxfinding

Problem

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 = 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 ]


-
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?

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

-
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

Solution

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:

  • Rename findMaxSequence to findMaxSequenceInner and remove the default values



  • 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 = 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 = 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 != 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.

Code Snippets

function findMaxSequence(array) {
    return findMaxSequenceInner(array, -1, -1, 0, []);
}

function findMaxSequenceInner(array, start, end, i, results){
    // ...
}
if((array.length == 1) || (array.length == 0)){
if (array.length <= 1) {
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); 
    }
}
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); 
}

Context

StackExchange Code Review Q#114060, answer score: 5

Revisions (0)

No revisions yet.