Angular way of keeping code dry in controller

Question

So I am building an Ionic app just to get familiar with it and it's my first time using Angular and i'm not quite in the Angular way of thinking yet.

I have two services, one that returns a location, and another that returns weather for that location. I have my data being organized in the service and send to the controller where I use the services function in the controller to set $scope variables for each of the data points to use in the view. I am wanting to integrate a 'pull to refresh' feature within the controller and for now I just have copied my code that was originally in the controller and wrapped it in a ion.Refresher directive. What is the best way of keeping this code DRY.

Thanks!

services.js

angular.module('weather.services', [])

.service('getLocation', function($http){
  this.getData = function(lat, long) {
    var promise = $http({
      method: 'GET',
      url: 'https://maps.googleapis.com/maps/api/geocode/json?latlng=' + lat + ',' + long
    }).success(function(data) {
      data.location = data.results[3].address_components[0].long_name;
      return data.location;
  });
  return promise;
 }
})

.service('getWeather', function($http){
  this.getData = function(lat, long) {
    var promise = $http({
        method: 'jsonp',
        url:  'https://api.forecast.io/forecast/api-key/' + lat + ',' + long + '?callback=JSON_CALLBACK'
    }).success(function(data) {
      data.currentTemperature = parseInt(data.currently.temperature);
      data.summary = data.currently.summary;
      data.icon = data.currently.icon;

      var timestamp = moment.unix(data.currently.time);
      var formattedDate = moment(timestamp, "YYYY-MM-DD HH:mm:ss")
      data.todaysDate = formattedDate.format('dddd Do MMMM');

      var daily_data = data.daily.data;
      var days = []
      angular.forEach(daily_data, function(value, key) {
        this.push(value);
      }, days);
      data.dailyWeather = days;

      // hourly weather for side scroll
      var hourly_data = data.hourly.data;
      var hours = []
      angular.forEach(hourly_data, function(value, key) {
       this.push(value);
      }, hours);
      data.hourlyWeather = hours;

      return data;
    });
    return promise;
   }
 })

controller.js

angular.module('weather.controllers', [])

.controller('HomeCtrl', function($scope, $http, getLocation, getWeather, $cordovaGeolocation) {

  $cordovaGeolocation
    .getCurrentPosition()
    .then(function (data) {
      $scope.latitude = data.coords.latitude;
      $scope.longitude = data.coords.longitude;

      // Gets location and city you are in
      getLocation.getData($scope.latitude, $scope.longitude).then(function(data){
        $scope.location = data.data.location;
      })

      // Gets the weather data for the city you are in
      getWeather.getData($scope.latitude, $scope.longitude).then(function(weather){
        $scope.weather = weather.data;
        $scope.today = weather.data.todaysDate;
        $scope.weather.currentTemp = weather.data.currentTemperature;
        $scope.weather.summary = weather.data.summary;
        $scope.weather.icon = weather.data.icon;
        $scope.weather.daily = weather.data.dailyWeather;
        $scope.weather.hourly = weather.data.hourlyWeather;
      })
    }, function(err) {
      console.debug(err);
    });

    // see the repeated code
    $scope.doRefresh = function() {
      console.log('currently refreshing data')
      $cordovaGeolocation
        .getCurrentPosition()
        .then(function (data) {
          $scope.latitude = data.coords.latitude;
          $scope.longitude = data.coords.longitude;

          // Gets location and city you are in
          getLocation.getData($scope.latitude, $scope.longitude).then(function(data){
            $scope.location = data.data.location;
          })

          // Gets the weather data for the city you are in
          getWeather.getData($scope.latitude, $scope.longitude).then(function(weather){
            $scope.weather = weather.data;
            $scope.today = weather.data.todaysDate;
            $scope.weather.currentTemp = weather.data.currentTemperature;
            $scope.weather.summary = weather.data.summary;
            $scope.weather.icon = weather.data.icon;
            $scope.weather.daily = weather.data.dailyWeather;
            $scope.weather.hourly = weather.data.hourlyWeather;
          })

          $scope.$broadcast('scroll.refreshComplete');
        }, function(err) {
          console.debug(err);
        });
    }
});

stop using success() it is deprecated (see $http docs). In your case chaining then() is more appropriate anyway and most of the processing code in your controller is better moved to the service. Then return one object to assign to $scope.weather — charlietfl
– charlietfl, Commented Sep 14, 2016 at 14:11
in fact if you kept the property names you use in your scope the same as what gets returned from the weather api it would cut down need to assign each property also which is a lot of extra work — charlietfl
– charlietfl, Commented Sep 14, 2016 at 14:39

Khiem Tran · Accepted Answer · 2016-09-14 14:19:08Z

1

You can do like this to keep your code DRY, it's the same as others:

.controller('HomeCtrl', function($scope, $http, getLocation, getWeather, $cordovaGeolocation) {

  $scope.loadData = function(isRefreshing) {
     ...// your code to load location and weather data here.
     //// 
     if (isRefreshing) {
        // broadcast your event
     }
  }

  $scope.loadData(); // load data for the first time
  $scope.doRefresh = function() {
     $scope.loadData(true); // load data on refreshing
  }

edited Sep 14, 2016 at 14:19

answered Sep 14, 2016 at 14:03

Khiem Tran

6,16916 silver badges25 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

charlietfl Over a year ago

all except for the event broadcast...which could be triggered based on argument in loadData()

Khiem Tran Over a year ago

what do you mean?

Khiem Tran Over a year ago

You just need to put your code that do the same thing in a function then call it instead of rewrite everything.

Khiem Tran Over a year ago

I see, put a boolean variable isRefreshing, if it's true then broadcast the event.

charlietfl Over a year ago

right but in one case an event is broadcast but not in the other

Collectives™ on Stack Overflow

Angular way of keeping code dry in controller

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related