Collecting a list of documents from a factory for an AngularJS controller

Question

With this controller I'm injecting myFactory and using Array.push() to add the array response.documents to the this.documents array. I can't help feeling there's a better way without having to use _this.

function myController($route, myFactory) {
  'ngInject';

  this.$onInit = function() {
    const _this = this;
    const id = $route.current.params.someId;

    this.documents = [];

    myFactory.getData(id).then((response) => {

      console.log(response); // { "something": { "Id":1234 }, "documents":[{ "DocumentId":"2345", "DocumentName":"doc.docx" }] }
      console.log(response.documents); // [{ "DocumentId":"2345", "DocumentName":"doc.docx" }]

      response.documents.forEach((object) => {
        _this.documents.push(object);
      });
    });
  };
}

Sᴀᴍ Onᴇᴌᴀ · Accepted Answer · 2018-02-24 00:39:06Z

Specify `this` context

I support Blindman67's answer. If arrow functions were not supported by all browsers used (e.g. IE - but then again there is little support for Angular in IE anyway) then you could also utilize Function.bind():

this.$onInit = function() {
    const id = $route.current.params.someId;

    this.documents = [];

    myFactory.getData(id).then((response) => {
        response.documents.forEach((object) => {
            this.documents.push(object);
        }.bind(this));
    }
}

Or utilize the second parameter of Array.foreach() (i.e. thisArg):

this.$onInit = function() {
    const id = $route.current.params.someId;

    this.documents = [];

    myFactory.getData(id).then((response) => {
        response.documents.forEach((object) => {
            this.documents.push(object);
          }, this);
    }
}

Avoid the looping

Even simpler would be to push all items with the spread operator, since Array.push() can accept a variable number of elements:

this.$onInit = function() {
    const id = $route.current.params.someId;

    this.documents = [];

    myFactory.getData(id).then(response => this.documents.push(...response.documents));
}

Because ...response.documents will spread the arguments out to match the parameters (i.e. element1[, ...[, elementN]]).

That way there is no need to iterate over the items. Array.concat() could also be used but then the array would need to be re-assigned. Or Array.unshift() could also be used with the spread operator.

Expand the snippet below for a demonstration (AngularJS code removed for simplicity).

const documents = [];
const id = 3;
const myFactory = {
  getData: function(id) {
    const newDocuments = [1,2,3];
    return new Promise(function(resolve, reject) {
      setTimeout(resolve, 750, {"documents": newDocuments});
    });
  }
};
console.log('original documents',documents);
myFactory.getData(id).then(function(response) {
  documents.push(...response.documents);
  console.log('documents after push',documents);
})

Very nice, I think it's only fair to update this as the accepted answer. The different examples and explanation for each helped me more than an hours searching and reading could. I'm going with the snippet you provided and the spread operator. Thanks Sam. — Wayne
– Wayne, Commented Feb 24, 2018 at 18:15

Marc Rohloff · Accepted Answer · 2018-02-23 16:35:26Z

2

In addition to what Blindman said, I would recommend avoiding using this in nested scopes , which is always confusing and error prone, by assigning documents to a local variable. i.e.

this.$onInit = function() {
  const id = $route.current.params.someId;
  let documents =this.documents = [];
  ...
  response.documents.forEach((object) => {
    documents.push(object);

answered Feb 23, 2018 at 16:35

Marc Rohloff

3,54710 silver badges7 bronze badges

Add a comment |

Blindman67 · Accepted Answer · 2018-02-23 14:14:00Z

1

Arrow functions are not bound to this so your code can use this.

this.$onInit = function() {
  const id = $route.current.params.someId;    
  this.documents = [];

  myFactory.getData(id).then((response) => {  // Arrow function
    response.documents.forEach((object) => {  // Arrow function

      this.documents.push(object);   // <<===== use this not _this

    });
  });
};

answered Feb 23, 2018 at 14:14

Blindman67

22.9k2 gold badges17 silver badges40 bronze badges

\$\begingroup\$ Ah of course, sometimes I can't see the wood for the trees! Thanks! \$\endgroup\$

Wayne
– Wayne

2018-02-23 14:19:39 +00:00
Commented Feb 23, 2018 at 14:19

Add a comment |

Stack Exchange Network

Collecting a list of documents from a factory for an AngularJS controller

3 Answers 3

Specify `this` context

Avoid the looping

You must log in to answer this question.

Hot Network Questions

Collecting a list of documents from a factory for an AngularJS controller

3 Answers 3

Specify this context

Avoid the looping

You must log in to answer this question.

Related

Hot Network Questions

Specify `this` context