7

In the following code, the actual length of user.roles is 1. However, the loop runs two times.

When I output the value of i, it is shown as 'diff' for the second iteration. Switching to the ordinary for loop resolved the situation. However, I'd like to know what the issue is with the for..in loop.

 for (var i in user.roles) {
                if (user.roles[i].school.equals(schoolId)) {
                    for (var j in user.roles[i].permissions) {
                        for (var k in accessType) {
                            if (user.roles[i].permissions[j].feature == featureKey) {
                                if (user.roles[i].permissions[j][accessType[k]]) {
                                    return true;
                                }
                            }
                        }
                    }
                }
            }

Update: user is an object, and roles is an array of objects. The instance of roles that caused the issue is shown below:

{
  "_id": "582d3390d572d05c1f028f53",
  "displayName": "Test Teacher Attendance",
  "gender": "Male",
  "roles": [
    {
      "_id": "57a1b3ccc71009c62a48a684",
      "school": "57a1b3ccc71009c62a48a682",
      "role": "Teacher",
      "__v": 0,
      "designation": true,
      "permissions": [
        {
          "feature": "User",
          "_id": "57ac0b9171b8f0b82befdb7d",
          "review": false,
          "view": true,
          "delete": false,
          "edit": false,
          "create": false
        },
        {
          "feature": "Notice",
          "_id": "57ac0b9171b8f0b82befdb7c",
          "review": false,
          "view": true,
          "delete": false,
          "edit": false,
          "create": false
        },

      ]
    }
  ],
}
Allen GJ
  • 83
  • 5
  • can you define user, roles, permission, accessType? those are objects, strings, integers. – Teocci Nov 17 '16 at 06:45
  • Is `user.roles` an Array/Iterator? Maybe you should use `for .. of` – zeronone Nov 17 '16 at 06:45
  • Why do you use `for in` ? try to use `forEach` . Its more convenient. http://stackoverflow.com/questions/23614054/javascript-nuances-of-myarray-foreach-vs-for-loop – Dhaval Soni Nov 17 '16 at 06:45
  • @Teocci user.roles is an array of objects, and permissions is an array of objects inside user.roles. accessType is a string array. – Allen GJ Nov 17 '16 at 07:28
  • @AllenGJ can you mark my question as the answer please? – Teocci Nov 18 '16 at 02:08

2 Answers2

3

user.roles seems to be an array. And for array you should not use for in.

Simple example

var arr = [2];
arr.s = 3;

for (var i  in arr) {
console.log("here"); // paints twice
}

From MDN, The for...in statement iterates over the enumerable properties of an object, in arbitrary order. For each distinct property, statements can be executed.

How to choose the type of iterator, here is a reference iterators

EDIT

As per the updated question, the above can only come with a property as diff if somewhere in the code following is present

Array.prototype.diff = .....
Nikhil Aggarwal
  • 28,197
  • 4
  • 43
  • 59
  • Thanks. However, in this case, user.roles only seems to have one enumberable property, right? ie, it only has one object in it. When I console the value of **i**, for the second (unexpected) iteration, it holds the value '**diff**'. – Allen GJ Nov 17 '16 at 08:05
  • If that is the case then there must be `Array.prototype.diff` in your code somewhere. – Nikhil Aggarwal Nov 17 '16 at 08:18
  • @AllenGJ - Did you checked? Additionally, you can delete it via `delete Array.prototype.diff;` – Nikhil Aggarwal Nov 17 '16 at 10:06
2

I think this is what are you looking for. I assume that your accessTypes is an array that contains the following items:

var accessTypes = ["review", "view", "delete", "edit", "create"];

Edited to improve the efficiency.

var schoolId = "57a1b3ccc71009c62a48a682";
var featureKey = "Notice";
var accessTypes = ["review", "view", "delete", "edit", "create"];

var user = {
  "_id": "582d3390d572d05c1f028f53",
  "displayName": "Test Teacher Attendance",
  "gender": "Male",
  "roles": [{
    "_id": "57a1b3ccc71009c62a48a684",
    "school": "57a1b3ccc71009c62a48a682",
    "role": "Teacher",
    "__v": 0,
    "designation": true,
    "permissions": [{
      "feature": "User",
      "_id": "57ac0b9171b8f0b82befdb7d",
      "review": false,
      "view": true,
      "delete": false,
      "edit": false,
      "create": false
    }, {
      "feature": "Notice",
      "_id": "57ac0b9171b8f0b82befdb7c",
      "review": false,
      "view": true,
      "delete": false,
      "edit": false,
      "create": false
    }]
  }]
};

user.roles.forEach(function(roleItem) {
  // console.log('This is a role: ' + roleItem.school);
  if (roleItem.school == schoolId) {
    roleItem.permissions.forEach(function(permissionItem) {
      // console.log('This is a permission: ' + permissionItem.feature);
      // console.log('This is a accessType: ' + accessType);
      if (permissionItem.feature == featureKey) {
        accessTypes.forEach(function(accessType) {
          if (permissionItem[accessType]) {
            console.log('accessType: ' + accessType + ' -> true');
            return true;
          }
        });
      }
    });
  }
});

forEach accepts an iterator function. The iterator function is called for each entry in the array, in order, skipping non-existent entries in sparse arrays.

forEach, also, has the benefit that you don't have to declare indexing and value variables in the containing scope, as they're supplied as arguments to the iteration function, and so nicely scoped to just that iteration.

If you're worried about the runtime cost of making a function call for each array entry, don't be; more technical details.

If you still have the impression that forEach is fundamentally slower you can use a simple for loop as I explain in another of my answers.

Hope that this helps you.

Community
  • 1
  • 1
Teocci
  • 7,189
  • 1
  • 50
  • 48
  • What about the performance? I was under the impression that forEach is fundamentally slower. – Allen GJ Nov 18 '16 at 03:56
  • @AllenGJ If you still have the impression that `forEach` is fundamentally slower you can use a simple `for` loop as I explain in another [Answer](http://stackoverflow.com/questions/40647538/#40647652). – Teocci Nov 18 '16 at 05:18