patternjavascriptMinor
Retrieving album artist info from third party API
Viewed 0 times
albuminfothirdpartyartistretrievingfromapi
Problem
I've recently just started with AngularJS and the best way for me to learn is to get some criticism to find out how I can do things better. I did a small project that calls a third party API and shows a bit of the return data.
HTML
Controller
What I'm specifically interested in knowing is:
To view the whole application feel free to view it on GitHub.
HTML
Get Albums
0">
Album Info
{{album.name}}
Controller
jamendoApp.controller('Album', function($scope, $http) {
$scope.getAlbums = function() {
$http.get("http://api.jamendo.com/v3.0/albums/?client_id=b6747d04&format=jsonpretty&artist_name=" + $scope.artist)
.success(function(data) {
if(data.headers.status == 'success') {
$scope.albums = data.results;
}
else {
alert("Error - " + data.headers.error_message);
}
})
.error(function(x, status, error) {
alert(error);
});
};
});What I'm specifically interested in knowing is:
- Am I writing the JavaScript for the controller correctly? It seems to work, but I can't help but feel like I'm not writing it the way actual JavaScript developers would.
- Am I using the AngularJS directives correctly in the HTML? I'm especially concerned if I'm using the ng-if as it's intended.
- Plus anything that may catch your eye that could be improved.
To view the whole application feel free to view it on GitHub.
Solution
Some pointers:
-
Move
-
-
Move
$http.get out of the controller to a service. Stuff that is not presentation logic should not be in your controller.-
getAlbums should probably be a method of the controller and not the $scope , see this video on egghead.io on how to use controller as syntax.- I don't really see the point in your
ng-if="albums.length > 0". Why is it there?
- It's common practice to postfix your controllers - so
AlbumControllerorAlbumCtrlare more common names. I usually try to stick to conventions like this to convey intent more clearly.
- If you're considering minification - consider using the minification syntax - start by reading this.
Context
StackExchange Code Review Q#32690, answer score: 2
Revisions (0)
No revisions yet.