0

I am trying to get a controller to retrieve its data from the server when it comes in to uses but for some reason this doesn't see to work properly:

app.controller('eventListController', ['$scope', '$http', '$routeParams', function ($scope, $http, $routeParams) {
    var eventList = this,
        getEventList = function () {
            var promise = $http.get('../json/Login.json');
            promise.then(function (response) {
                eventList = response.data;
            }, function (error) {
                window.alert('Error' + error);
            });
        };

    getEventList();
}]);

It seems pretty straightforward but eventList doesn't load correctly

What I am doing wrong ?

Here is roughly what the JSON looks like 

{
"eventHead": [
    {
        stuff stuff
    },
    {
         more stuff
    }
],
"success": true

}

if i do a 

window.alert(eventList);

after the

getEventList();

I get [object Object], which seems normal

but if I do 

window.alert(eventList.success);

I get undefined

and furthermore my data just doesn't load into the page

Grégoire Sion
  • 53
  • 9

3 Answers3

2

You don't want to overwrite the reference from this (your controller) with the result (EDIT: What i mean is, you're not longer referencing the controller and instead just referencing the data - which will not be accessible in the view). You want to set a property on the controller - i take it you're using the controller as syntax? This would do a better job at what I think you're trying to achieve:

app.controller('eventListController', ['$scope', '$http', '$routeParams', function ($scope, $http, $routeParams) {
    var that = this;

    var getEventList = function () {
        var promise = $http.get('../json/Login.json');
        promise.then(function (response) {
            that.eventList = response.data;
        }, function (error) {
            console.log('Error', error);
        });
    };

    getEventList();
}]);

EDIT: It has been pointed out to me several times, that the above (question) syntax is correct. I agree - it's just bad practice. You should not be defining several variables with , and newlines. I think most javascript developers would agree that it does not add to readability (I think me mis-reading it proves that).

Per Hornshøj-Schierbeck
  • 15,097
  • 21
  • 80
  • 101
  • The syntax is correct and you don't override `this` with the result. – martin Jul 08 '16 at 12:29
  • @Martin Perhaps my wording is a bit off, but the syntax is not correct. You can hold a reference to the controller, but you need to set the data on a property on that controller, or you won't be able to access it in the view since you will have no reference.... – Per Hornshøj-Schierbeck Jul 08 '16 at 12:33
  • `var a, b, c;` is perfectly valid. what error do you see in this line? – Jai Jul 08 '16 at 12:36
  • I dont think the syntax is quite off, because Brackets gives me and error message if I try to use var = something two times, even if it ends up working, but ill post the solution i came up with thnaks to your answer below – Grégoire Sion Jul 08 '16 at 12:39
  • @PerHornshøj-Schierbeck Even if you wanted to show it in a view you'd have to use `$scope.eventList` and not `this.eventList` – martin Jul 08 '16 at 12:41
  • @GrégoireSion The error (not working part) in your code was not the syntax obviously - it just made it hard for me to understand - the error part was not putting the data on the controller, but instead on a local variable. I demonstrated that in my code and "cleaned" up the syntax that threw me off – Per Hornshøj-Schierbeck Jul 08 '16 at 12:41
  • @Martin not if you were using the controller-as syntax, then you set it on this/controller – Per Hornshøj-Schierbeck Jul 08 '16 at 12:42
0

Your problem is actually well described in questions such as: How do I return the response from an asynchronous call?

Calling window.alert(eventList); does return [object Object] because you assigned it to eventList = this.

Function getEventList() is asynchronous so the response isn't available immediately. You could do:

var getEventList = function (callback) {
    var promise = $http.get('../json/Login.json');
    promise.then(function (response) {
        eventList = response.data;
        callback(eventList);
    }, function (error) {
        window.alert('Error' + error);
    });
};

getEventList(function(response) {
    console.log(response.success);
});

But rather have a look at the answer I mentioned above and see how to use Promises in Angular.

Community
  • 1
  • 1
martin
  • 93,354
  • 25
  • 191
  • 226
  • I think you're overcomplicating it. All you have to do was set the data on either $scope if you're using scope or on a property on the controller like i described. – Per Hornshøj-Schierbeck Jul 08 '16 at 12:43
0

Thumbs up to Per Hornshoj-Schierbeck and Martin who set me in the right direction , this seems to work

app.controller('eventListController', ['$scope', '$http', '$routeParams', function ($scope, $http, $routeParams) {
    var events= this;

    var getEventList = function () {
        var promise = $http.get('../json/Login.json');
         promise.then(function (response) {
            events.eventList = response.data;
        }, function (error) {
            window.alert('Error' + error);
        });
     };

    getEventList();
}]);

I hope the syntax is good practice, if not ill be glad to receive any advice

Grégoire Sion
  • 53
  • 9
  • looks like i have to wait two days – Grégoire Sion Jul 08 '16 at 12:48
  • 1
    About the syntax. I would put a semi-colon after this on the first line and then define a new variable for getEventList. It's easier to read when you only define one variable per line. Also loose the indent of the getEventList function. It should be on the same indent as the two other lines. Check out my response again if you want to see what i mean. – Per Hornshøj-Schierbeck Jul 08 '16 at 12:55
  • the weird thing about that is that brackets gives me a message when i try it : "combine this with the previous 'var' statement" – Grégoire Sion Jul 08 '16 at 12:57
  • @GrégoireSion you should be able to accept my answer after 15minutes wait. You only have to wait 48 hours if you're accepting your own answer. Also about brackets, i haven't tried it, i just know that combining the variables doesn't add to readability. – Per Hornshøj-Schierbeck Jul 08 '16 at 13:10
  • 1
    Also a final comment. Instead of naming your variable 'events', why not name it 'me' or 'that' - both are widely used for capturing this in a different/parent scope, which is what you're doing. It makes it very explicit what you are referencing. You could also name it 'controller' since it is the controller itself that you're holding a reference to – Per Hornshøj-Schierbeck Jul 08 '16 at 13:12
  • will do ! thank you for this, I lack knowledge of common practice – Grégoire Sion Jul 08 '16 at 13:17
  • @GrégoireSion I have to ask because i'm curious. Are you planning on marking this (your own answer) the as the accepted one - is that why you said you have to wait two days, or does it tell you that you also have to wait two days before accepting my answer? – Per Hornshøj-Schierbeck Jul 08 '16 at 13:31
  • Hum i don't really know how all this works I am new on stack overflow but if you want ill tag your answer as accepted, it's valid anyway – Grégoire Sion Jul 08 '16 at 13:43
  • 1
    That is how this forum work. You mark the answer you think is the best as accepted. You can mark your own answer, but that is only done if none of the other answers prove correct. It also identifies the question as 'closed' - no need to add more answers... – Per Hornshøj-Schierbeck Jul 08 '16 at 13:57
  • okay thank you ! I must have looked selfish to try to upvote myself – Grégoire Sion Jul 08 '16 at 14:12