Lessons Learned

State

vs.

Pure Functions

State

  • Result depends on it

function Adder(first, second) {
  this.first = first;
  this.second = second;

  this.sum = function () {
    return this.first + this.second;
  }
}

var adder = new Adder(2, 3);
adder.sum();

State

  • Introduces an additional parameter: TIME

function Adder(first, second) {
  ...

  this.setFirst = function(first) {
    this.first = first;
  }

  this.setSecond = function(second) {
    this.second = second;
  }
}

var adder = new Adder(2, 3);
... //black hole :P
adder.sum(); // =?

Those who don't play nice

function badGuy(param) {
  param.first = 10;
  param.second = -1;
}

var adder = new Adder(2, 3);
badGuy(adder) //i don't know what bad guy does
adder.sum(); // =5, right? :P
  • don't mutate the parameters inside a function (seriously, don't!)

Pure functions

function Adder() {
  this.sum = function(first, second) {
    return first + second;
  }
}

var adder = new Adder();
adder.sum(2, 3); // =5, always!
  • no state

  • depend only on parameters

  • play very nice with immutable objects

Case Study

vm.toggleOptions = { 
  nextAvailableDate: nextDate,
  label: "today", 
  today: moment.utc().startOf('day').toDate() 
};

function toggleStartDateValue() {
  vm.toggleOptions.toggleCheck = (moment.utc().startOf('day').isSame(vm.value));
  vm.toggleOptions.label = (vm.toggleOptions.toggleCheck) ? 
                            "today" : 
                            "next available date";
  vm.value = (vm.toggleOptions.toggleCheck) ? 
              vm.toggleOptions.nextAvailableDate : 
              vm.toggleOptions.today;
}

Case Study

vm.toggleOptions = { 
  nextAvailableDate: nextDate,
  label: "today", 
  today: moment.utc().startOf('day').toDate() 
};

function toggleStartDateValue() {
  # check if vm.value is today
  vm.toggleOptions.toggleCheck = moment(vm.toggleOptions.today).utc().isSame(vm.value);
  # check if the date picker reads today and vm.value was manually set to today
  vm.toggleOptions.datepickerToday = vm.toggleOptions.label === "today" && 
                                     vm.toggleOptions.toggleCheck;
  
  if (vm.toggleOptions.datepickerToday) {
    vm.toggleOptions.label = "next available date";
    vm.value = vm.toggleOptions.today;
  } else {
    vm.toggleOptions.label = (vm.toggleOptions.toggleCheck) ? 
                             "today" : 
                             "next available date";
    vm.value = (vm.toggleOptions.toggleCheck) ? 
                                  vm.toggleOptions.nextAvailableDate : 
                                  vm.toggleOptions.today;
  }
}

Issues?

  • state is vm.value... + the label => complexity!

  • what is the public interface?

    • should only be the label, but a lot is exposed

Functional approach

vm.toggleStartDateValue = toggleStartDateValue;
vm.toggleLabel = toggleLabel;
var nextAvailableDate = nextDate;

function toggleStartDateValue() {
  vm.value = isToday() ? nextAvailableDate : today().toDate();
}

function toggleLabel() {
  return isToday() ? 'next available date' : 'today';
}

function isToday() {
  return (today().isSame(vm.value));
}
function today() {
  return moment.utc().startOf('day');
}

Mutating state

function formatStats(stats) {

  stats.chart = {

    labels: ['<2 weeks', '<4 weeks'],

    colors: ['#cc3a3a', '#f4aa16'],

    data: [stats.global.lessThanTwoWeeks, stats.global.lessThanFourWeeks],

    options: {responsive: true, maintainAspectRatio: true}
  
  };


  stats.byLocation = R.reduce(function (acc, locationInfo) {
    
    return R.merge(acc, R.objOf(locationInfo.location.code, locationInfo));
  
  }, {}, stats.byLocation);

  
  return stats;

}
describe('directive', function () {
  var stats;
  
  beforeEach(function () {
    
    stats = formatStats(insuranceReportStats);

    ...
    element = compile('<div insurance-report-stats="stats"></div>')(scope);

    scope.$digest();
  });

  it('test something', function () {...});
  it('test something else', function () {...});
  it('test something another thing', function () {...});
});

Exposing state

  • open the door

  • go to the third room on the left

  • turn right

  • open the fridge door

  • look on the second shelf

  • move the milk bottle

  • take one of the beer cans

  • enjoy!

Give me a beer

Give me a beer

  • ring the door bell

  • ask for a beer

  • wait

  • here you go, enjoy!

function FleetStatusStats() {
  var directive = {
    scope: {},
    bindToController: {
      stats: '=fleetStatusStats',
    },
    controllerAs: 'vm',
    controller: FleetStatusStatsCtrl
  };

  return directive;
}
<div ...>
  {{ vm.stats.global.total }}
</div>

<div ...>
  {{ vm.stats.chartRevenue.data }}
</div>

<li ng-repeat="chunk in vm.stats.byLocation.chunks">
  <div ...>
    {{ chunk[0].location.stats.fullyMissionCapable }}
  </div>
</li>
it('should have correct values', function () {
  var text = element.text();
  expect(text).toContain('Active Aircraft');
  expect(text).toContain('N-Rev');
  expect(text).toContain('Rev');
  expect(text).toContain('Bagram');
  expect(text).toContain('Malta');
  expect(text).toContain('3 FMC');
  expect(text).toContain('2 FMC');
  expect(text).toContain('4 NMC');
  expect(text).toContain('1 NMC');
});

Testing

vs.

function FleetStatusStats() {
  var directive = {
    scope: {},
    bindToController: {
      stats: '=fleetStatusStats',
    },
    controllerAs: 'vm',
    controller: FleetStatusStatsCtrl
  };

  return directive;
}

function FleetStatusStatsCtrl() {
  function locationName(code) {
    return vm.stats.byLocation[code].location.name;
  }

  function statsForLocation(code) {
    return vm.stats.byLocation[code].stats;
  }

  ...
}
<div ...>
  {{ vm.globalStats().total }}
</div>

<div ...>
  {{ vm.revenueChart() }}
</div>

<li ng-repeat="chunk in vm.chunks">
  <div ...>
    {{ vm.statsForLocation(chunk[0]).fullyMissionCapable }}
  </div>
</li>
it('should return the correct stats', function () {
  expect(ctrl.statsForLocation('foo')).toEqual({...});
});

it('should have the correct chunks', function () {
  expect(ctrl.chunks).toEqual([...]);
});

Testing

Interfaces

Make it obvious

function algHudInsuranceReportDetails() {
  return {
    restrict: 'A',
    scope: {
      stats: '=algHudInsuranceReportDetails'
    }
  };
}

function AlgHudInsuranceReportDetailsCtrl(stats) {
  var vm = this;

  vm.stats = stats;
  vm.getInsuranceStatsByLocation = getInsuranceStatsByLocation;

  function getInsuranceStatsByLocation() {
    return vm.stats ? vm.stats.insuranceReportStats.byCountry.chunks : [];
  }
}

Make it obvious

  • accept only what you need

  • simplifies your code

  • simplifies the caller's life :D

  • simplifies refactorings

  • no need to navigate btw controller and view to see what data is used

  • simplifies testing (a lot!)

Testing

  • test the entire interface, not parts

function api(item) {
  item.whatINeed = computeIt(item);
  return item;
}

// tested like this:

it('...', function () {
  var input = {
    useful: ...,
    blah: ...
  };

  var result = api(input);
  expect(result.whatINeed).toEqual(...);

  // what about result.useful, result.blah ?
  // are they needed, how can api() 
  // be refactored?
});
function api(item) {
  return {
    whatINeed: computeIt(item)
  };
}

// tested like this:

it('...', function () {
  var input = {
    useful: ...,
    blah: ...
  };

  expect(api(input)).toEqual(...);
});

More on testing

Tests = code

  • stubbing / mocking​

    • creates a fake environment

    • isolation vs. real life

    • reusable service vs. implementation details

  • a lot of stubbing might be a "code smell"
  • stubs which are not present in source code
    • bad tests?
    • bad code?

Naming convention

it('should return aog-report-new-bg for stats.newNMCs !=0', ...);
it('should return aog-report-discussed-bg for stats.discussedNMCs !== 0 && stats.newNMCs === 0',
 ...);
it('should return aog-report-none-bg for stats.discussedNMCs === 0 && stats.newNMCs === 0',
 ...);
  • talk about the use, not the implementation
it('should return aog-report-new-bg if the location has new parts', ...);
it('should return aog-report-discussed-bg if the location has only discussed parts', ...);
it('should return aog-report-none-bg if the location has no parts', ...);

Testing... nothing :)

var ... (long list of mocked stuff), stats, ...;

describe('directive', function () {
  beforeEach(function () {
    
    $scope = $rootScope.$new();

    
    $scope.stats = { ... };
    stats = $scope.stats;
  };



  it('test something', function () {...});
});

describe('controller', function () {
  beforeEach(function() {
    $scope = $rootScope.$new();

 
    $scope.stats = stats;
  });

  it('test something but with no stats!', function () {
    expect(ctrl).toBeDefined();
  });
});

Testing... nothing :)

​Before

  • HTML would have nothing in it (sausage expressions fail silently)

​After

  • test interface methods (fail)

Testing... nothing :)

beforeEach(function () {
    
  $scope = $rootScope.$new();

    
  $scope.stats = {
 ... }; // big fat object here :D
  element = $compile('<div alg-hud-fleet-status="summary"></div>')($scope);

  $scope.$digest();
});

Last but definitely not least

The root of all (most) evil is...

COPY-PASTE

What to do?

  • understand the source of inspiration
  • do I need it? is it generic?

    • extract reusable component/utility

  • improve the code to improve the test
  • don't let LOC guide you
  • be proactive :D
  • most of the copy pasted code is mocking code :)
  • backend test code - the requires

lessons learned

By Horia Radu

lessons learned

  • 312