2
\$\begingroup\$

Does the following code is written correct witout redundant code ? or maybe using Jquery it can be written better?

I need to find matching between properties insdie oData & oPushObj and fill aSelData with this content.

This code is working as expected

        jQuery.each(aDataCollection, function(iIndex, oData) {
            oPushedObject = {};

            fnAddEnt(aProperties, oData, oPushObj);

            Object.getOwnPropertyNames(oData).forEach(function(key) {
                if (oPushObj.hasOwnProperty(key)  {

                    var source = oData[key].results[iIndex];
                    var destination = oPushObj[key];

                    Object.getOwnPropertyNames(source).forEach(function(sourceKey) {
                        if (destination.hasOwnProperty(sourceKey)) {
                            var sourceItem = source[sourceKey];
                            destination[sourceKey] = sourceItem;
                        }
                    });
                }
            });

            aSelData.push(oPushObj);
        });
        return aSelData;
    };
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

General.

I notice that this question is a new expression of the other one you asked previously... but have now deleted!

In the previous question you didn't really explained something that becomes clear with this new one: you're looking for matching properties (with unknown names) successively at two nesting levels. In addition, 1st step works at level 1 of each object, while 2nd step works at level 2 for one object and level 3 for the other.

It's not a good idea to have deleted your previous question because, in the other hand, it showed an example of the involved objects structure. Since I remembered that, I could understand what your current code does (and BTW, understand what was not previously clear).
At the opposite, for somebody reading your current question, and without the knowing of the structure, it's pretty hard to see what your code is intended to.

About the current code.

The part of code you posted is inconsistent:

  • Its 2 last lines show that it's part of some loop, whose beginning is missing.
  • Yet at the beginning, it also lacks at least a declaration var aSelData = [];.
  • The assignation oPushedObject = {}; is out of date: this object is named oPushObj in the rest of the code.

Technically speaking, there are 3 points to notice:

  • Apart from its erroneous name, the above cited assignation should be var oPushObj = {}; in order not to pollute the global space.
  • Object.getOwnPropertyNames() is not the best way to get the involved properties, because it returns all properties, both enumerable or not, and with the prototype's one added: merely use Object.keys().
  • Likewise, if (oPushObj.hasOwnProperty(key) (note it lacks a closing parenthesis) is overkill where if (oPushObj[key]) is enough.

Note (thanks to @Roamer-1888 for pointing it): this last suggestion is pertinent only if the oPushObh[key] can't contain any falsy value.

Taking in account all the remarks above, here is a modified version of your code, where I also added some comments to explain how it works:

var aSelData = [];
jQuery.each(aDataCollection, function(iIndex, oData) {
    var oPushObj = {};
    // Here oPushObj gets populated
    fnAddEnt(aProperties, oData, oPushObj);
    // Look for matching properties between oPushObj and oData (1st level)
    Object.keys(oData).forEach(function(key) {
        if (oPushObj[key]) {
            // Look for matching properties between oPushObj[key] (2nd level)
            // and oData[key][iIndex] (3rd level)
            var source = oData[key].results[iIndex];
            var destination = oPushObj[key];
            Object.keys(source).forEach(function(sourceKey) {
                if (destination.[sourceKey]) {
                    // Populate oPushObj[key][sourceKey] value
                    destination[sourceKey] = source[sourceKey];
                }
            });
        }
    });
    aSelData.push(oPushObj);
});

Suggested alternate way

You currently use the simple approach of iterating through objects, for each level of the search process. And sure this works.

But you might also consider a different approach, based on extracting the intersection of the objects properties, successively for the 2 involved levels.
IMO the code is slightly more readable this way, making its main part look as a single-line one.
In the other hand, I couldn't assert it's faster.

Here is it:

var aSelData = [];
jQuery.each(aDataCollection, function(iIndex, oData) {
    var oPushObj = {};
    // Here oPushObj gets populated
    fnAddEnt(aProperties, oData, oPushObj);
    // Look for matching properties between oPushObj and oData (1st level)
    for (var key of arrayIntersect(
      Object.keys(oPushObj), Object.keys(oData)
    )) {
        // Look for matching properties between oPushObj[key] (2nd level)
        // and oData[key][iIndex] (3rd level)
        for (var sourceKey of arrayIntersect(
          Object.keys(oPushObj[key]), Object.keys(oData[key][iIndex])
        )) {
            // Populate oPushObj[key][sourceKey] value
            oPushObj[key][sourceKey] = oData[key][iIndex][sourceKey];
        }
    }
    aSelData.push(oPushObj);
});

function arrayIntersect(a, b) {
  return a.filter(key => b.indexOf(key) > -1)
}
\$\endgroup\$
3
  • \$\begingroup\$ With regard to if (oPushObj[key]) as a substitute for if (oPushObj.hasOwnProperty(key), what if the key exists but its value is falsy (false, null, NaN, empty string)? \$\endgroup\$ Commented Mar 20, 2016 at 6:20
  • \$\begingroup\$ @Roamer-1888 From a general point of view, you're totally right. In the current case, the original question I cited in my answer showed examples whose contents seemed to exclude this type of situation. It's why I proposed this short way here. So actually it's up to the OP to choose the good one. Anyway I'll update my answer to precise this point. \$\endgroup\$
    – cFreed
    Commented Mar 20, 2016 at 15:54
  • \$\begingroup\$ @cFreed - THanks a lot! I've new question which is a bit similar to this with bounty please see if you can help here either ,Thanks stackoverflow.com/questions/36062777/… \$\endgroup\$
    – 07_05_GuyT
    Commented Mar 20, 2016 at 20:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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