Determine whether corresponding elements of two arrays can be swapped to make both arrays increasing

Question

I've managed to create a function that checks whether arrays 'a' and 'b' can 'swap' corresponding values to 'increase' - e.g.:

var a = [1,2,3,4,5] // is increasing
var a = [1,4,6,7,36] // is increasing
var a = [1,6,3,6,5] // not increasing

Some successful tests:

increaser([5,3,7,7],[1,6,6,9]) - should return 2; a = [1,3,6,7] & b = [5,6,7,9]
increaser([1,5,6],[-2,0,2]) - should return 0; already increasing
increaser([5,-3,6,4,8],[2,6,-5,1,0]) - should return -1; cannot increase
increaser([5,3,7,7,10],[1,6,6,9,9]) - should return 3; a = [1,3,6,7,9], b = [5,6,7,9,10]
increaser([2,1,2,3,4,5],[0,1,7,5,2,3]) - should return -1; cannot increase

And the function returns 0 if the values in each array are 'already increasing', and returns -1 if the values can't be swapped in order to increase.

However, I belive I've written way overcomplex code, particularly for the parts that push checks if the conditions allow the array values to swap, and pushes the new values into the array - e.g.:

if(((b[i] >= b[i+1] || b[i] <= finalB[i-1]) && (a[i] < b[i+1] && a[i] > finalB[i-1]) && (b[i] < a[i+1]) && b[i] > finalA[i-1]) && ((a[i] >= a[i+1] || a[i] <= finalA[i-1]) && (b[i] < a[i+1] && b[i] > finalA[i-1])))

So my question is, how could I have written this to be much simpler? Any advice on writing better code would also be appreciated, as I'm relatively new to JavaScript. Thanks a lot

The solution below:

function increaser(a, b) {

  function isAlreadyIncreasing(c, i, arr) { // c = currentValue; i = index; arr = array
    return ((i == arr.length - 1 && c > arr[i - 1]) || c < arr[i + 1]);
  }

  if (!(a.every(isAlreadyIncreasing) && b.every(isAlreadyIncreasing))) {
    console.log('Arrays are not already increasing');

    // continue with function

    var finalA = [], finalB = [];
    var counter = 0; // no. of changes

    // for every element, check if each element needs changing
    for (i = 0; i < a.length; i++) {

      // for first elements, if conditions match, push elements into new array
      if (i == 0) {
        if ((a[0] >= a[1] && b[0] < a[1] && a[0] < b[1]) || 
        (b[0] >= b[1] && a[0] < b[1] && b[0] < a[1])) {
          finalA.push(b[0]);
          finalB.push(a[0]);
          counter++;
        } else {
          finalA.push(a[0]);
          finalB.push(b[0]);
        }
      }

      // for 2nd - n-1th elements
      if (i > 0 && i < a.length - 1) {
        if (((b[i] >= b[i + 1] || b[i] <= finalB[i - 1]) && 
        (a[i] < b[i + 1] && a[i] > finalB[i - 1]) && 
        (b[i] < a[i + 1]) && b[i] > finalA[i - 1]) && 
        ((a[i] >= a[i + 1] || a[i] <= finalA[i - 1]) && 
        (b[i] < a[i + 1] && b[i] > finalA[i - 1]))) {
          finalA.push(b[i]);
          finalB.push(a[i]);
          counter++;
        } else {
          finalA.push(a[i]);
          finalB.push(b[i]);
        }
      }

      // for last elements, if conditions match, push elements into new array
      if (i == a.length - 1) {
        if ((a[i] <= finalA[i - 1] && b[i] > finalA[i - 1] && a[i] > finalB[i - 1]) || 
        (b[i] <= finalB[i - 1] && a[i] > finalB[i - 1] && b[i] > finalA[i - 1])) {
          finalA.push(b[i]);
          finalB.push(a[i]);
          counter++;
        } else {
          finalA.push(a[i]);
          finalB.push(b[i]);
        }
      }
    }
    console.log("Original A: " + a);
    console.log("Original B: " + b);
    console.log("Final A: " + finalA);
    console.log("Final B: " + finalB);
    console.log("Counter: " + counter);

    // if counter is 0, return -1, otherwise return counter
    if (counter == 0) {
      console.log('But arrays cannot increase');
      return -1;
    } else {
      return counter;
    }
  } else {
    console.log('Arrays are already increasing');
    return 0;
  }
}

Answer 1

Style

• Don't add an else after condition blocks that terminate (returns or throws) current context execution:

  if (foo) {
      return;
  } else {  // else and block are superfluous
      blah = 0;
  }
  
  // the same as above but with less noise
  if (foo) {
      return;
  }
  blah = 0;
  
  // or
  if (foo) { return }
  blah = 0;
  

• Don't use == or !=; use === or !== instead

• Don't test for known states. For instance, if iteration variable i starts at 0 you know i > 0 if i !== 0.

    if (i == 0) {
      //... code
    }  /* There should be an else here */
  
    // for 2nd - n-1th elements
    if (i > 0 && i < a.length - 1) {
  

  Can be

  if (i === 0) {
      //... code
  } else if (i < a.length - 1) {  // already know i > 0
  

• Don't add comments that state the obvious (such as // for 2nd - n-1th elements)

• Don't add unnecessary adverbs to names. For instance, isAlreadyIncreasing can be isIncreasing.

Logic and efficiency

• Avoid repetitive indexing into arrays. Use local scoped variables to reduce the amount of indexing. Reduce the chance of typo errors and increase readability.

Without changing the logic (it is flawed) the inner loop can be written as:

for (i = 0; i < a.length; i++) {
  const a0 = a[i], a1 = a[i + 1];
  const b0 = a[i], b1 = a[i + 1];
  if (i == 0) {
    if ((a0 >= a1 && b0 < a1 && a0 < b1) || (b0 >= b1 && a0 < b1 && b0 < a1)) {
      finalA.push(b0);
      finalB.push(a0);
      counter++;
    } else {
      finalA.push(a0);
      finalB.push(b0);
    }
  } else {
    const fa = finalA[i - 1]
    const fb = finalB[i - 1];
    if (i < a.length - 1) {
      if (((b0 >= b1 || b0 <= fb) && (a0 < b1 && a0 > fb) && 
      (b0 < a1) && b0 > fa) && ((a0 >= a1 || a0 <= fa) && (b0 < a1 && b0 > fa))) {
        finalA.push(b0);
        finalB.push(a0);
        counter++;
      } else {
        finalA.push(a0);
        finalB.push(b0);
      }
    } else {
      if ((a0 <= fa && b0 > fa && a0 > fb) || (b0 <= fb && a0 > fb && b0 > fa)) {
        finalA.push(b0);
        finalB.push(a0);
        counter++;
      } else {
        finalA.push(a0);
        finalB.push(b0);
      }
    }
  }
}

• Don't continue processing when a definite solution has been determined (increaser([5,-3,6,4,8],[2,6,-5,1,0])) at the second element. It can be deduced that both arrays cannot increase. Thus, the code should exit with a result. You can use break to exit from a loop.

• Be aware of edge cases. Your code assumes that both arrays have content, but what happens if the arrays are empty or have only one item? Does your code still give the correct answer?

• Try reduce the number of times you you touch each item of data. In the worst case you iterate over both arrays twice, but in many cases you need only iterate part of both arrays. For instance, increaser([5,-3,6,4,8],[2,6,-5,1,0]) need only touch the first and second item of both arrays to know the result. You do 15 iterations, while only 2 are required.

Rewrite

My brain is a little tender so there may be a much better solution.

The rewrite tries to reduce the amount of work by avoiding the initial test that determines if the arrays are increasing. A single while loop will do the same while testing for swaps.

As soon as it finds a condition that does not let both arrays increase (and there is no rollback option) then it terminates with the result -1.

There does not seem to be any reason to look ahead, but there does seem to be a need to rollback if swapping or keeping values both increase the arrays and later there is no solution. Hence, rollback keeps the first point that the iteration can roll back to if a solution does not work. (NOTE that rollback is always set to the first item). Also, rollbackResult keeps the number of swaps up until that points.

I have only tested on the inputs given and can't be bothered with more at the moment.

For arrays that differ in size or are empty this throws a range error, and if the length of the arrays are 1 then returns an undefined result as the question does not state what should happen in this case.

Your test for increaser([5,3,7,7,10],[1,6,6,9,9]). You have a result of 3, yet it can be done in 2 (No need to swap the first item):

function increaser(arr1, arr2){
    var rollback, rollbackResult, result, i;
    if (arr1.length !== arr2.length || arr1.length === 0) { throw new RangeError("Bad input") }
    if (arr1.length === 1) {
        console.log("Question is ambiguous, the answer is undefined.")
        return;  // undefined result
    }

    const res1 = [], res2 = [];
    i = result = 0;

    while (i < arr1.length) {
        const p1 = res1[i - 1], p2 = res2[i - 1];
        let a1 = arr1[i], a2 = arr2[i];
        const canSwap = i === 0 || (p1 < a2 && p2 < a1);
        if (i === 0 || (p1 < a1 && p2 < a2)) {
            if (rollback === undefined && canSwap) { 
                rollback = i;
                rollbackResult = result;
            }
        } else if(canSwap) { 
            [a2, a1] = [a1, a2];
            result += 1;
        } else {
            if (rollback !== undefined) {
                res2.length = res1.length = i = rollback;
                a1 = arr2[i];
                a2 = arr1[i];
                rollback = undefined;
                result = rollbackResult + 1;
            } else {
                result = -1;
                break;
            }
        }
        res1.push(a1);
        res2.push(a2)
        i += 1;
    }
    return result;
}

Answer 2

One way to improve the code would be to format the really long conditional statements which seem completely unreadable so they are visually structured.

For example, change

// for 2nd - n-1th elements
    if (i > 0 && i < a.length-1) {
        if(((b[i] >= b[i+1] || b[i] <= finalB[i-1]) && (a[i] < b[i+1] && a[i] > finalB[i-1]) && (b[i] < a[i+1]) && b[i] > finalA[i-1]) && ((a[i] >= a[i+1] || a[i] <= finalA[i-1]) && (b[i] < a[i+1] && b[i] > finalA[i-1]))) {

to

// for 2nd - n-1th elements
if (i > 0 && i < a.length-1) {
    if (  (  (
               b[i] >= b[i+1]      ||
               b[i] <= finalB[i-1]
             )
            &&
             (
               a[i] < b[i+1]       &&
               a[i] > finalB[i-1]
             )
            &&
             b[i] < a[i+1]
            &&
             b[i] > finalA[i-1]
          )
         &&
          (  (
               a[i] >= a[i+1]      ||
               a[i] <= finalA[i-1]
             )
            &&
             (
               b[i] < a[i+1]       &&
               b[i] > finalA[i-1]
             )
          ) 
       )
    {