0
\$\begingroup\$

I wrote code to search by keyword, and it will check all of the values from the object property, including an array in the object. But apparently the code is not optimized as it takes some time to execute the function, and it is subject to client PC processing power.

Here is the sample of the object:

{
    clientId: "-LFQ1ojwCF8lSRq4J9sQ",
    desc: "description lorus ipsum etc",
    document: [
      {
        doc: "photo",
        status: "ok",
        url: "https://www.google.com"
      }
    ],
    dt: "2018-06-24",
    dtactual: "2018-06-26",
    dtcreate: 1529905260612,
    dtexpected: "2018-06-25",
    dtmodify: 1529905260612,
    dtreceive: "2018-06-26",
    engineer: "engineer 001",
    equipment: [
      { brand: "ruckus", imei: 123, imsi: 312, model: "R230-1220v6HW", serialNo: 123},
      { brand: "", imei: "", imsi: "", model: "", serialNo: ""},
      { brand: "", imei: "", imsi: "", model: "", serialNo: ""},
      { brand: "", imei: "", imsi: "", model: "", serialNo: ""},
      { brand: "", imei: "", imsi: "", model: "", serialNo: ""}
    ],
    iwoNum: "iwo num 2",
    key: "-LFpT2ajia6Wgnrd8lQH",
    onSiteSupport: { isSupport: true, supportTerm: "" },
    pm: "project manager 001",
    poNum: "po no 002",
    preventMaintenance: true,
    projectType: "site installation",
    remark: "iwo remark",
  }

Imagine there are 50 or 100, or 1000 of the object in an array, and the user wishes to list the object that contains a specific keyword. So the code need to search every property to find the value.

So below is my code (written in Typescript, but almost same with JS):

iwoLit = []
objectArray = [{...},{...},{...},...] 
//the details of the object in this array please refer to above object as it is too long to place at here.
//Feel free to change any value in there.
keyword = 'ruckus'

searchFunction(){
  objectArray.forEach(element => {
    let object = Object.assign({}, element);
    object['key'] = element.key;
    let keys = Object.keys(object)
    for (let i = 0; i < keys.length; i++) {
      let prop = keys[i]
      let item = object[prop]
      if (typeof item == 'string') {
        let item2 = item.toString()
        let item3 = item2.toLowerCase()

        if (item3.indexOf(this.keyword.toLowerCase()) !== -1) {
          if (this.iwoList.length == 0) {
            this.iwoList.push(object);
          }
          else {
            for (let l = 0; l < this.iwoList.length; l++) {
              if (object.key !== this.iwoList[l].key) {
                this.iwoList.push(object);
                return false
              }
            }
          }
        }

      }
      //to handle the array in the object
      let equipValue3
      if (Array.isArray(item)) {
        item.forEach(element2 => {
          let itemKeys = Object.keys(element2)
          for (let j = 0; j < itemKeys.length; j++) {
            let prop2 = itemKeys[j]
            let equipValue = element2[prop2]
            if (typeof equipValue == 'string' || typeof equipValue == 'number') {
              let equipValue2 = equipValue.toString()
              equipValue3 = equipValue2.toLowerCase()
              if (equipValue3.indexOf(this.keyword.toLowerCase()) !== -1) {
                if (this.iwoList.length == 0) {
                  this.iwoList.push(object);
                }
                else {
                  for (let k = 0; k < this.iwoList.length; k++) {
                    if (object.key !== this.iwoList[k].key) {
                      this.iwoList.push(object);
                      return false
                    }
                  }
                }
              }
            }
          }
        })
      }
    }
  });
  console.log(this.iwoList)
}

Would be appreciated if someone can help me optimize the code. It is ok to separate in few functions, as long as the performance can be increased.

Side info: Why do I want to make such a nested object? Because I am using noSQL, and this is the DB design.

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

Too much messing about in hyper space.

You have way too much messing about. Object.assign is slow and more so when you don't need it.

RegExp is fast

If you are going to search strings use a RegExp it is far far quicker. And if not don't convert the same value to lower (or upper) case every time you need to check it. Once is all you need to do.

I can not quite workout where you are getting element.key from.

Set and Map for lookups

Rather than search manually over an array of keys you can use a Set that will provide fast hash table lookup

// at the very start of the search create an empty set
var foundKeys = new Set(); 

...

// then rather than loop over iwoList check if the set already has the key
if (!foundKeys.has(object.key)) { 
    foundKeys.add(object.key);  // if not add the new one
    this.iwoList.push(object);
}
// saving a mass of cycles.

Look for the quick answer

It would be better to check the foundKeys set right at the start before you search the object. No point searching for a match only to dump the result because you have that key. Make it the first thing to check before you search the object, first thing inside the loop would be the best way to avoid many cycles.

Bloat also eats cycles

Its all a little hanging out and bloated at the start.

 let object = Object.assign({}, element);  // object is copied from element
 object['key'] = element.key;  // then you copy key, but key will be in object from the line above
 let keys = Object.keys(object)   // then create a keys array and a iterator index
 for (let i = 0; i < keys.length; i++) {
   let prop = keys[i]  // another variable 
   let item = object[prop]  // and another

Why not just the one line, same as the 6 above just much quicker...

for(const item of Object.values(element)){

The rewrite

Without more info, I read your code to find the first instance of the search term and add the object that it was part of from the objectArray to the result.

Checking arrays, and objects, and matching any strings or numbers found, case insensitive.

As a standalone function the following returns an array of objects that contain a property that matches keyword. I would assume that the objects come from a JSON so have ignored any cyclic references problems (which can be checked with a Set if needed)

The string matching is via a regExp

The whole thing is just an extended Array.filter call

function search(array, keyword) {
  const regExp = new RegExp(keyword,"gi");
  const check = obj => {
    if (obj !== null && typeof obj === "object") { return Object.values(obj).some(check) }
    if (Array.isArray(obj)) { return obj.some(check) }
    return (typeof obj === "string" || typeof obj === "number") && regExp.test(obj);
  }
  return array.filter(check);
}
\$\endgroup\$
  • \$\begingroup\$ Blindman67, Thanks for providing such informative reply and also the rewrite. Do you mind to explain how the each line of the code is working? I can roughly understand how it work but i believe your explanation can help me understand more.. Your code apparently is much higher level than mine (a lot). Thanks again. \$\endgroup\$ – Jerry Jun 29 '18 at 18:13
  • \$\begingroup\$ In my typescript it show below message for obj in the regExp.test(obj): Argument of type 'string | number' is not assignable to parameter of type 'string'. Type 'number' is not assignable to type 'string'. \$\endgroup\$ – Jerry Jul 1 '18 at 18:44
0
\$\begingroup\$

As always I suggest to use functional approach, which makes code more readable and concise. I wrote my version, which uses recursion to search through all properties. Each of present functions has single responsibility, which simplifies maintaining of the code. The code is written using ES6.

const objectArray = [{...},{...},{...},...]

console.log(findFromList(objectArray, 'ruckus'));

function findFromList(list, keyword) {
    return list.filter((el) => search(el, keyword));
}

function search(el, keyword) {
    const type = Array.isArray(el) ? 'array' : typeof el;
    const searchFunc = getFuncByType(type);

    return searchFunc(el, keyword);
}

function getFuncByType(type) {
    const match = {
        'string': searchInText,
        'number': searchInText,
        'boolean': searchInText,
        'array': searchInArray,
        'object': searchInObject,
    };

    if (typeof match[type] !== 'undefined') {
        return match[type];
    } else {
        throw new Error(`Unknown element type "${type}"`);
    }
}

function searchInText(text, keyword) {
    return (text.toString().indexOf(keyword) !== -1);
}

function searchInObject(obj, keyword) {
    return searchInArray(Object.values(obj), keyword);
}

function searchInArray(arr, keyword) {
    return arr.find((el) => search(el, keyword)) !== undefined;
}
\$\endgroup\$
  • \$\begingroup\$ Thanks Stexxe, the code is much tidy and better. \$\endgroup\$ – Jerry Jun 29 '18 at 18:06

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.