2
\$\begingroup\$

I'm pretty new to Angular. I wrote a weather app and need my code reviewed. I especially need help on best practices for handling edge cases on the response from the JSON endpoint.

Here's the HTML:

<!DOCTYPE html>
<html>

<head>
    <meta charset="utf-8">
    <meta name="viewport" content="initial-scale=1, maximum-scale=1, user-scalable=no, width=device-width">
    <title></title>

    <link href="lib/ionic/css/ionic.css" rel="stylesheet">
    <link href="css/style.css" rel="stylesheet">

    <!-- IF using Sass (run gulp sass first), then uncomment below and remove the CSS includes above
    <link href="css/ionic.app.css" rel="stylesheet">
    -->

    <!-- ionic/angularjs js -->
    <script src="lib/ionic/js/ionic.bundle.js"></script>

    <!-- cordova script (this will be a 404 during development) -->
    <script src="cordova.js"></script>

    <!-- your app's js -->
    <script src="js/app.js"></script>
</head>
<body ng-app="starter" ng-controller="apiController">

    <ion-pane>
        <ion-header-bar class="bar-calm">
            <h1 class="title">Kansas City Weather</h1>
        </ion-header-bar>
        <ion-content>
            <div ng-repeat="x in myData">
                <div class="card">

                    <div class="item item-divider">
                        Day {{ $index + 1 }}
                    </div>


                    <div class="row">
                        <div class="col col-75">
                            <div class="item item-text-wrap">
                                {{ x.temp.day }} &#8451;
                            </div>

                        </div>
                        <div class="col col-25">
                            <div class="row">
                                Max : {{ x.temp.max }} &#8451;
                            </div>
                            <div class="row">
                                Min : {{ x.temp.min }} &#8451;
                            </div>

                        </div>
                    </div>
                </div>   
            </div>
        </ion-content>
    </ion-pane>
</body>

</html>

Here's the app.js :

// Ionic Starter App

// angular.module is a global place for creating, registering and retrieving Angular modules
// 'starter' is the name of this angular module example (also set in a <body> attribute in index.html)
// the 2nd parameter is an array of 'requires'
var app = angular.module('starter', ['ionic'])

    .run(function($ionicPlatform) {
    $ionicPlatform.ready(function() {
    // Hide the accessory bar by default (remove this to show the accessory bar                  above the keyboard
    // for form inputs)
    if(window.cordova && window.cordova.plugins.Keyboard) {
      cordova.plugins.Keyboard.hideKeyboardAccessoryBar(true);
    }
    if(window.StatusBar) {
      StatusBar.styleDefault();
    }
    });
    })


      app.controller('apiController', function($scope, $http) {
    $http.get("http://api.openweathermap.org/data/2.5/forecast/daly?q=Kansas+City&mode=json&units=metric&cnt=10&appid=f6b7081abd94e66273817ed6b5ce95c7").then(function(response) {
        $scope.myData = response.data.list;
    });
});
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

Your JS could need improvement in terms of indentation. Other than that, your app is pretty small for a full-blown review. However... we can make improvements.

In the long run, your app will be more than just one controller to display different parts of the site. Most of them will be consuming more or less the same data, just displayed a bit differently. You might want to consider using factories/services as data stores, and have controllers just pull them in as dependencies.

This also means your API calls will live there as well, freeing the controller to only be a means to expose data from the services/factories to the templates as well a communicate from template to services/factories.

\$\endgroup\$

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.