1
\$\begingroup\$

Hi I am working on a complex application and currently adding two additional properties to objects that match the condition and returning it back. I was able to achieve the expected output, but just wanted to know if there is any other way which is more efficient than what I have tried.

Step 1 : Filtering the array to get only car objects.

Step 2 : Add firstlayer and lastlayer prop to car objects

Step 3 :

  • If it is first car object in the filtered array then mark firstlayer as true. If it is last car object in the filtered array then mark lastlayer as true.

Step 4 : concat it with the other non-car objects

enter image description here

Original array:

[{
  id: 10,
  name: "Truck",
  status: "Cancelled"
}, {
  id: 11,
  name: "Bus",
  status: "Approved"
}, {
  id: 12,
  name: "Car1",
  status: "Approved"
}, {
  id: 19,
  name: "Car2",
  status: "Cancelled"
}, {
  id: 13,
  name: "Car3",
  status: "Cancelled"
}]

Expected array:

[{
      id: 10,
      name: "Truck",
      status: "Cancelled"
    }, {
      id: 11,
      name: "Bus",
      status: "Approved"
    }, {
      id: 12,
      isFirstLayer: true,
      isLastLayer: false,
      name: "Car1",
      status: "Approved"
    }, {
      id: 19,
      isFirstLayer: false,
      isLastLayer: false,
      name: "Car2",
      status: "Cancelled"
    }, {
      id: 13,
      isFirstLayer: false,
      isLastLayer: true,
      name: "Car3",
      status: "Cancelled"
    }]

This is the code I have tried

var array = [
  {
    "name": "Truck",
    "status": "Cancelled",
    "id": 10
  },
  {
    "name": "Bus",
    "status": "Approved",
    "id": 11
  },
  {
    "name": "Car1",
    "status": "Approved",
    "id": 12
  },
  {
    "name": "Car2",
    "status": "Cancelled",
    "id": 19
  },
  {
    "name": "Car3",
    "status": "Cancelled",
    "id": 13
  }
];

var i=0;
var arrayLength=array.length;
var newArray = array.filter(function(a) {
  if((a.name !='Bus'&&a.name!='Truck')) 
  {   
      
      if(i==0){
		  a.isFirstLayer=true;
          a.isLastLayer=false;
      }else if(i==arrayLength-i-1){
      	  a.isFirstLayer=false;
          a.isLastLayer=true;
      } else{
          a.isFirstLayer=false;
          a.isLastLayer=false;
      }
      i++;
  };
  return a
});

console.log(newArray);

https://jsfiddle.net/fierce_trailblazer/q2uvf06r/24/

Any suggestions to improve the code would be helpful

\$\endgroup\$
1
\$\begingroup\$

Some things I noticed:

Array.prototype.filter is intended to be used when you want to filter out elements from an array and construct a new array from certain elements of the other array, for example:

const arr = [0, 1, 2, 3, 4];
const evens = arr.filter(num => num % 2 === 0);
console.log(evens);

If you're not taking elements out to make a new array, then .filter isn't appropriate. If you're just looking to iterateover an array and perform side-effects (like mutate certain elements), use Array.prototype.forEach or for..of instead.

That said - I think using filter would be the best way to approach this problem, by separating out the cars from non-cars. More on that later.

You have

if((a.name !='Bus'&&a.name!='Truck')) 
if(i==0){
}else if(i==arrayLength-i-1){

It would be better to never use sloppy comparison with == and != - instead, use strict comparison with === and !==. The problem with sloppy comparison is that it has so many weird rules a developer has to memorize in order to be confident in its proper usage. See this image. If you're comparing items of the same type, using strict comparison instead will work just fine. (When comparing items of different types, you can check their types and then explicitly cast to the same type if needed, then perform the comparison.)

I'd also recommend adding spaces between operators - it improves code readability. (Many decent IDEs can do this automatically.) Eg, compare

if((a.name !='Bus'&&a.name!='Truck')) 

to

if (a.name !== 'Bus' && a.name !== 'Truck')

(There's no need for the extra ()s around the condition)

An even better method might be to check whether the item's name starts with Car:

if (a.name.startsWith('Car'))

You're missing a semicolon with return a. Occasionally forgetting semicolons can result in confusing bugs, especially if you aren't an expert. Consider using a linter to automatically prompt you to fix these sorts of potential mistakes.

If I were approaching this problem, I'd prefer to separate out the cars into their own array. This separation can be achieved concisely with Array.prototype.filter. Then, iterate over the cars, and add the isFirstLayer and isLastLayer properties (as false) to each car. Then, take the first car and set its isFirstLayer to true. Similarly, take the last car and set its isLastLayer to true:

var array = [{
    "name": "Truck",
    "status": "Cancelled",
    "id": 10
  },
  {
    "name": "Bus",
    "status": "Approved",
    "id": 11
  },
  {
    "name": "Car1",
    "status": "Approved",
    "id": 12
  },
  {
    "name": "Car2",
    "status": "Cancelled",
    "id": 19
  },
  {
    "name": "Car3",
    "status": "Cancelled",
    "id": 13
  }
];
const cars = array.filter(item => item.name.startsWith('Car'));
for (const car of cars) {
  car.isFirstLayer = false;
  car.isLastLayer = false;
}
cars[0].isFirstLayer = true;
cars[cars.length - 1].isLastLayer = true;
console.log(array);

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for the review :) \$\endgroup\$ – Akshay Jun 6 at 5:48
3
\$\begingroup\$

It may be a better idea to use a reducer to initially add the properties to a subset of the elements of the array. Finally you can change the last element from the new Array with a property isLastLayer. I used Array.find within the reducer lambda and a custom function findLast for finding the last array-element with property isLastLayer.

const result = document.querySelector("#result");
const log = (...txt) => result.textContent += txt.join("\n") + "\n";

// moved to function, so the relevant code is more visible
const array = getTestArray();

// find the last element using the predicate lambda
const findLast = (array, predicate) => {
  for (let i = array.length - 1; i >= 0; --i) {
    if (predicate(array[i])) {
      return array[i];
    }
  }
};

// reduce the values, adding the layer props to 
// all elements where the name does not start 
// with 'bus' or 'truck'. Use Array.find within the 
// accumulated array to determine if isFirstLayer
// is already in the set of elements
let newArray = array.reduce((acc, value) => {
  if (/^(bus|truck)/i.test(value.name)) {
    return [...acc, value];
  }
  const isFirst = !acc.find(el => "isFirstLayer" in el);
  value = { ...value,
    isFirstLayer: isFirst,
    isLastLayer: false
  };
  return [...acc, value];
}, []);

// use findLast to find the last element with
// a property 'isLastLayer' in the array, and 
// change its value
findLast(newArray, el => "isLastLayer" in el).isLastLayer = true;

// let's see if it worked
log(JSON.stringify(newArray, null, 2));

function getTestArray() {
  return [{
      "name": "Truck",
      "status": "Cancelled",
      "id": 10
    },
    {
      "name": "Bus",
      "status": "Approved",
      "id": 11
    },
    {
      "name": "Car1",
      "status": "Approved",
      "id": 12
    },
    {
      "name": "Car2",
      "status": "Cancelled",
      "id": 19
    },
    {
      "name": "Car3",
      "status": "Cancelled",
      "id": 13
    }
  ];
}
<pre id="result"></pre>

It can also be done without the extra findLast function

const result = document.querySelector("#result");
const log = (...txt) => result.textContent += txt.join("\n") + "\n";
const array = getTestArray();
let accumulator = {
  newArr: [], 
  // track first/last
  firstLayerSet: false,
  lastLastLayer: null
};
const addPropsReducer = (acc, value, i) => {
  if (/^(bus|truck)$/i.test(value.name)) {
    return {
      newArr: [...acc.newArr, value]
    };
  }
  value = { ...value,
    isFirstLayer: !acc.firstLayerSet,
    isLastLayer: false
  };
  return {
    ...acc,
    newArr: [...acc.newArr, value],
    firstLayerSet: true,
    lastLayerIndex: i
  };
};

let newArray = array.reduce(addPropsReducer, accumulator);
newArray.newArr[newArray.lastLayerIndex].isLastLayer = true;

// ditch tracking values
newArray = newArray.newArr;

log(JSON.stringify(newArray, null, 2));

function getTestArray() {
  return [{
      "name": "Truck",
      "status": "Cancelled",
      "id": 10
    },
    {
      "name": "Bus",
      "status": "Approved",
      "id": 11
    },
    {
      "name": "Car1",
      "status": "Approved",
      "id": 12
    },
    {
      "name": "Car2",
      "status": "Cancelled",
      "id": 19
    },
    {
      "name": "Car3",
      "status": "Cancelled",
      "id": 13
    }
  ];
}
<pre id="result"></pre>

Play with this code in jsFiddle.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ Thanks for the review :) \$\endgroup\$ – Akshay Jun 6 at 5:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.