comments and feedback

.eslintrc

@@ -1,3 +1,5 @@
+// you probably want to extend eslint recommended or at least put
+// in no unused vars and other syntax help
 {
     "parserOptions": {
 		"ecmaVersion": 6,

src/app.js

@@ -28,6 +28,7 @@ app.config(routes);
 
 const dev = 'https://pawsio.herokuapp.com/api';
 
+// need to use the DefinePlugin or EnvironmentPlugin in webpack to manage url
 // app.value('apiUrl', process.env.API_URL || '/api');
 app.value('apiUrl', dev);
 app.config(http);

src/components/all-pets/all-pets.js

@@ -14,6 +14,10 @@ controller.$inject = [ 'petsService', '$state' ];
 function controller(petsService, $state){
     this.styles = styles;
 
+    // better to use a single object:
+    // this.reset = () => this.pet = {};
+    // and let the bindings create the properties
+
     this.reset = () => {
         this.name = '';
         this.age = '';
@@ -26,6 +30,7 @@ function controller(petsService, $state){
 
     this.reset();
 
+    // should come from db?
     this.breeds = [
       { breed: 'Working Dog', exerciseNeed: 120 },
       { breed: 'Terrier', exerciseNeed: 110 },
@@ -40,8 +45,6 @@ function controller(petsService, $state){
 
     this.addPet = function(){
 
-        let breedName = '';
-        let exercise = null;
         let petToAdd = {
             name: this.name,
             age: this.age,
@@ -54,8 +57,14 @@ function controller(petsService, $state){
         petsService.addPet(petToAdd)
           .then(savedPet => {
               this.pets.push(savedPet);
+              // 3. better to just do here, synchronous activity:
+              this.reset();
           })
-          .then(this.reset());
+          // 1. this calls the reset function and passes the return
+          // as the callback :(
+          // .then(this.reset());
+          // 2. this is what you meant:
+          // .then(() => this.reset());
     };
 
 }

src/components/pet/pet.js

@@ -18,7 +18,11 @@ function controller(petsService, $state, snapService) {
     this.styles = styles;
 
     this.$onInit = function () {
+        // time to work on model/variable naming when
+        // you have meanless names like this that only describe data structure
         let dataArr = this.petData.data;
+
+        // Data formatting logic could be moved into custom filter
         for (var i = 0; i < dataArr.length; i++) {
             let longDate = dataArr[i].date;
             let dateChunk = longDate.slice(0, 10);

src/components/snapshot/snapshot.js

@@ -47,8 +47,11 @@ function controller(kineticsService, petSnapshotService, temperatureService, use
     this.$onInit = function () {
         console.log('distanceGoal: ', this.pet.distanceGoal);
         let temp = temperatureService.getAvgTemp(this.snapshot.dataPayload);
+        // why isn't this part of the getAvgTemp service method?
         this.averageTemp = Math.round(temp);
 
+        // hmm, this seems odd here. Logic seems duplicative of some of services.
+        // I wonder if data on server should be better massaged
         this.snapshot.dataPayload.forEach((element, index, array) => { 
             this.threshold.push(element.threshold);
             this.time.push((Date.parse(element.date) - Date.parse(array[0].date))/1000); 
@@ -59,6 +62,7 @@ function controller(kineticsService, petSnapshotService, temperatureService, use
         kineticsService
             .getVelocity(this.snapshot.dataPayload)
             .then(velArr => {
+                // again, not sure why this isn't part of service
                 let rawHikeLength = velArr[(velArr.length) - 1].timeStamp;
                 if ((rawHikeLength/60) < .5) {
                     this.hikeLengthMin = Math.ceil(rawHikeLength/60);

src/components/stats/stats.js

@@ -1,6 +1,8 @@
 import template from './stats.html';
 import styles from './stats.scss';
 
+// duplicate of "profile" component ????
+
 export default {
     template,
     controller,

src/routes.js

@@ -12,6 +12,7 @@ export default function routes($stateProvider, $urlRouterProvider) {
             pets: ['petsService', pets => {
                 return pets.getAll()
                   .then(pets => {
+                      // pets.pets? sounds like model should be cleaned up or service should hide this bit...
                       return pets.pets;
                   });
             }]
@@ -39,6 +40,10 @@ export default function routes($stateProvider, $urlRouterProvider) {
         abstract: true,
         default: '.pets',
         resolve: {
+            // since 'about', 'profile', and 'snapshot' use the same pets data,
+            // you could create single top level 'app' state that applies to both.
+            // as you currently have it, there are two pets arrays in memory and
+            // you loose syncing data across staes 
             pets: ['petsService', pets => {
                 return pets.getAll()
                   .then(pets => {

src/services/kinetics-service.js

@@ -1,3 +1,5 @@
+// these calculations seem deterministic. Could they be done
+// on save of data on the server?
 export default function kineticsService() {
     return {
         getVelocity(accelArray) {

webpack.config.js

@@ -2,6 +2,7 @@ const HtmlWebpackPlugin = require('html-webpack-plugin');
 var ExtractTextPlugin = require('extract-text-webpack-plugin');
 var CopyWebpackPlugin = require('copy-webpack-plugin');
 
+// either use this instance or stick with static methods, but don't mix the two
 const cssExtract = new ExtractTextPlugin('main.css');
 
 module.exports = {