patternjavascriptMinor
Finding the max sequence finder
Viewed 0 times
thefindersequencemaxfinding
Problem
Problem Statement
Find the max sequence finder.
Expected Output
Solution
Actual output
-
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
Note: Above problem is pulled from here. Tested on firefox browser, Chrome could not
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]noSolution
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:
Like this:
Instead of this:
It would be better to combine those conditions:
When all branches of if-else-if-else conditions return, you can simplify to multiple ifs, like this:
Looking at this part:
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:
Once again, nesting level is reduced, which is easier to read.
Lastly, there are some formatting issues:
In the above examples I already adjusted the indenting and spacing, I hope you see the difference from your original.
I think it's good to write code that runs anywhere.
You can make this code run anywhere with a simple modification:
- Rename
findMaxSequencetofindMaxSequenceInnerand remove the default values
- Create a new
findMaxSequencefunction
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.