HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Retrieving album artist info from third party API

Submitted by: @import:stackexchange-codereview··
0
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


    
      
      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 $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 AlbumController or AlbumCtrl are 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.