How spaghetti is this?

let diffArray = (arr1, arr2) => {
  let newArr = arr1.concat(arr2); //To make it easier to see duplicates.
  let ocur = [];
  let cut = []; 
  let diff = [];
  let getOc = (arr, value) => {
    return arr.filter((v) => (v === value)).length;
        }; //Counts the occurrences of certain value in an array.
    for (let i = 0; i < newArr.length; i++) {
      if (typeof newArr[i] == "number") { //Kept getting weird outputs for number arrays and string arrays, so I sorted them by their type.
        newArr.sort((a, b) => a - b);
          } else {
            newArr.sort();
          }
ocur.push(getOc(newArr, newArr[i])); 
//Makes an array with the occurrences in the same index as newArray.
    };
      while (ocur.includes(1)) { //Splices the values that have an occurrence of 1.
        cut = newArr.splice(ocur.indexOf(1), 1);
        diff = diff.concat(cut);
        ocur.splice(ocur.indexOf(1), 1);
  } 
  return diff;
}

It’s clunky compared to the other solutions. How spaghetti is it, and what do you think could improve it? (Using the same method of counting value occurrences and etc?) Or do away with it all? Thank you for reading, your time is appreciated!

Could you link the problem you’re working on? I can already see some basic things like cut and diff could be combined into 1 thing and then returned without even assigning it to a variable.

Sure!

One thing that would help make it much more readable, would be to indent your code sections properly. See below for your code indented in a more readable way.

let diffArray = (arr1, arr2) => {
  let newArr = arr1.concat(arr2); //To make it easier to see duplicates.
  let ocur = [];
  let cut = [];
  let diff = [];
  let getOc = (arr, value) => {
    return arr.filter((v) => (v === value)).length;
  }; //Counts the occurrences of certain value in an array.
  
  for (let i = 0; i < newArr.length; i++) {
    if (typeof newArr[i] == "number") { //Kept getting weird outputs for number arrays and string arrays, so I sorted them by their type.
      newArr.sort((a, b) => a - b);
    } else {
      newArr.sort();
    }
    ocur.push(getOc(newArr, newArr[i]));
    //Makes an array with the occurrences in the same index as newArray.
  };
  
  while (ocur.includes(1)) { //Splices the values that have an occurrence of 1.
    cut = newArr.splice(ocur.indexOf(1), 1);
    diff = diff.concat(cut);
    ocur.splice(ocur.indexOf(1), 1);
  }
  return diff;
}
1 Like

Yeah, you seem to have done it in a pretty roundabout and complicated way. I suggest you look at solution 3 from that link to see how simple the problem really was. I used to do the same thing where I would store each step in a different variable but its really not needed and adds a lot of extra characters / variables to your code, complicating it for anyone else who may need to work on it.

1 Like

Thanks! That is much better.

Thanks! I guess just doing away with it is best if there’s something as simple as that lol

1 Like