Using chartJS in Angularjs to plot IOT data

Question

I am a Javascript and AngularJS beginner.

Below is the code inside my chartController. What this does is get IOT data from a back-end call and display it in my front-end with a chart. There are two helper functions for the chart: addData() and remove() to add and remove data. These functions make it so that the chart gets new data to the right of the chart and removes data on the left. This is what I have so far. It would be appreciated if someone would give me tips on how:

• to make my controller less stacked, get data -> draw chart -> update chart.
• to make the get method more generic. The URL is now hardcoded.

var vm = this;
vm.content = [];
vm.dataContent = [];

$http({
  method: 'GET',
  url: 'http://localhost:8080/data/Living Room'
})
  .then(function successCallback(response) {

    JSON.stringify(response.data);

    angular.forEach($scope.content, function (value) {
      vm.dataContent.push(value.inputData);

    });

    $scope.content = response.data;

    var canvas = document.getElementById("myChart");
    var ctx = canvas.getContext("2d");

    var dataChart = {
      labels: ["January", "February", "March", "April", "May", "June", "July"],
      datasets: [{
        label: "Testing my Set",
        data: vm.dataContent
      }]
    };;


    var settings = {
      type: "line",
      data: dataChart
    };

    var myNewChart = new Chart(ctx, settings);


    function addData(chart, data, label) {
      chart.data.labels.push(label);
      chart.data.datasets.forEach(function (dataset) {
        dataset.data.push(data);
      });
      chart.update();

    }

    function remove(chart) {
      chart.data.labels.shift();
      chart.data.datasets.forEach(function (dataset) {
        dataset.data.shift();
      });
      chart.update();
    }

    setInterval( function(){

      addData(myNewChart, 16,  'working on it');

      // Remove the first point so we dont just add values forever
      remove(myNewChart);
    }, 5000);



  }, function errorCallback(response) {
    return alert(response.status)
  });

Answer 1

Feedback

The code appears to make good use of AngularJS and ChartJS, as well as promises.

For the function that handles the response, I had to move the line that assigns response.data to $scope.content up before the call to angular.forEach(). Otherwise it seems that the data wouldn't get assigned before the iteration...

Also I don't see the point of calling JSON.stringify() on the response.data object unless that gets assigned to a variable or passed to another function...

function handleResponse(response) {
    //what is the point of this line? the return value isn't assigned to a variable or passed to a function
    JSON.stringify(response.data);
    $scope.content = response.data;            
    angular.forEach($scope.content, function (value) {
        vm.dataContent.push(value.inputData);
    });
}

Recommendations

Below are my recommendations. They are by no means a definitive list of improvements but will hopefully be useful in terms of readability, flow of the code, etc. In general, I aim to avoid lengthy functions - with ~10-20 lines or fewer. For more guidance, refer to answers on this post.

Making separate functions

• to make my controller less stacked, get data -> draw chart -> update chart.

First, I would move the implementation of the successCallback() and errorCallback() functions out to be defined later. Hoisting can be utilized here so the functions can be defined after being called, though one could also move the definitions up if desired. And if there was a need to call the functions multiple times, then the functions could be referenced simply by name in each instance.

$http({
    method: 'GET',
    url: 'http://localhost:8080/data/Living Room'
})
    .then(successCallback, errorCallback); 

function successCallback(response) {
    JSON.stringify(response.data);
    //skipping the rest of the implementation
}
function errorCallback(response) {
    return alert(response.status);
}

Then I would break out the blocks within successCallback into separate functions. Even if they aren't called in multiple places, it can improve readability. While there are varying opinions, many people argue methods should be kept between 5-15 lines (refer to this answer - I know that is tagged with java and c# but it can apply to many other languages as well).

In the example below, I made one function to handle taking the response, and a separate function to get the settings passed to the chart constructor.

function successCallback(response) {
    handleResponse(response);

    var canvas = document.getElementById("myChart");
    var ctx = canvas.getContext("2d");

    var myNewChart = new Chart(ctx, getSettings());
    setInterval( function(){
        addData(myNewChart, 16,  'working on it');
        // Remove the first point so we dont just add values forever
        remove(myNewChart);
    }, 5000);
};
function handleResponse() { ... }
function getSettings() { ... }

Making URL variable

• to make the get method more generic. The URL is now hardcoded.

There are various ways to make the URL a variable. One approach is to have it be controlled by user interaction. For example, the UI could have inputs bound to the url via ngBind. And another approach might be to have specific URLs defined in the controller and reference those URLs. So in the controller, we could define:

$scope.urls = {
    livingRoom: 'http://localhost:8080/data/Living Room,'
    hallway: 'http://localhost:8080/data/Hallway'
}; //add more elements as appropriate

Then in the UI, have buttons like:

<button ng-click="loadData(urls.livingRoom)">Living Room</button>
<button ng-click="loadData(urls.hallway)">Hallway</button>

And define that function loadData like below, moving the call to $http() into it:

$scope.loadData = function(url) { 
    if (interval) { //check if interval is updating data periodically
        clearInterval(interval);
    }
    $http({
        method: 'GET',
        url: url //use variable value based on parameter from button
    }).then(successCallback, errorCallback);  
};

You may notice that function begins by checking if (interval) and if that evaluates to true then it will call clearInterval() passing that value. That will handle clearing the previous interval, as long as we assign the return value from setInterval() to that value:

var interval; //set scope outside functions
function successCallback(response) {
    //...handle response
    //...create chart
    interval = setInterval(function() {...},5000);
}

Demonstration

See a demonstration here in this phpfiddle.

Conclusion

If you look at the functions in that phpFiddle example, then you will notice that the longest function (excluding the controller callback) is ~9 lines long. This greatly enhances readbility.

JS error on hover

I noticed that after loading data from one source, then reloading with another source, when hovering over the chart I would see an error in my console:

Uncaught TypeError: Cannot read property 'skip' of undefined

That appears to be this reported ChartJS bug.