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);
 });
 });
 };
}

Answer 1

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);
})

• Wayne: 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.

Answer 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);

Answer 3

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
 });
 });
};

• Wayne: Ah of course, sometimes I can't see the wood for the trees! Thanks!