1
\$\begingroup\$

This a basic filter I created with Angular that does temperature conversion. I'd like to get thoughts on how to improve this code from both an Angular and general JS perspective.

The filter should take 2 arguments:

  • scale: The temperature scale to convert to
  • label: Whether to append a degree symbol to the number

In the view:

// should output: 99
{{ 37 | formatTemperature: 'F' }}

// should output: 99°
{{ 37 | formatTemperature: 'F':true }}

// should output: 37
{{ 98.6 | formatTemperature: 'C' }}

The filter:

(function() {
  'use strict';

  angular.module('example')
    .filter('formatTemperature', [
      function() {
        return function(input, scale, label) {
          var value = parseInt(input, 10),
              convertedValue;

          if (isNaN(value)) throw new Error('Input is not a number');

          if (scale === 'F') {
            convertedValue = Math.round(value * 9.0 / 5.0 + 32);
          } else if (scale === 'C') {
            convertedValue = Math.round((value - 32) * 5.0 / 9.0);
          } else {
            throw new Error('Not a valid scale');
          }

          return label ? convertedValue += '\u00B0' : convertedValue;
        };
      }
    ]);
})();

Testing the filter:

(function() {
  'use strict';

  describe('formatTemperature test', function() {
    beforeEach(module('example'));

    var filter;

    beforeEach(inject(function(_$filter_) {
      filter = _$filter_('formatTemperature');
    }));

    it('should convert temperature from Celsius to Fahrenheit', function() {
      expect(filter(37, 'F')).toEqual(99);
    });

    it('should convert temperature from Fahrenheit to Celsius', function() {
      expect(filter(98.6, 'C')).toEqual(37);
    });

    it('should append a % to the converted value', function() {
      expect(filter(37, 'F', true)).toEqual('99\u00B0');
    });

    it('should throw an error if the scale is invalid', function() {
      expect(function() {
        filter(37, 'G');
      }).toThrow(new Error('Not a valid scale'));
    });

    it('should throw an error if value is not a number', function() {
      expect(function() {
        filter('test', 'F');
      }).toThrow(new Error('Input is not a number'));
    });
  });

})();
\$\endgroup\$
2
\$\begingroup\$

On the principal that functions should do one thing and do it well, consider breaking out some pieces of your filter into smaller functions for readability.

Filter

(function () {
    'use strict';

    angular.module('example')
        .filter('formatTemperature', formatTemperatureFilter);

    var degreesSymbol = '\u00B0';

    function convertCelsiusToFahrenheit(value) {
        return Math.round(value * 9.0 / 5.0 + 32);
    }

    function convertFahrenheitToCelsius(value) {
        return Math.round((value - 32) * 5.0 / 9.0);
    }

    function addDegreesSymbol(value) {
        return value += degreesSymbol;
    }

    function formatTemperatureFilter() {
        return function (input, scale, label) {
            var value = parseInt(input, 10),
                convertedValue;

            if (isNaN(value)) throw new Error('Input is not a number');

            if (scale === 'F') {
                convertedValue = convertCelsiusToFahrenheit(value);
            } else if (scale === 'C') {
                convertedValue = convertFahrenheitToCelsius(value);
            } else {
                throw new Error('Not a valid scale');
            }

            return label ? addDegreesSymbol(convertedValue) : convertedValue;
        };
    }

})();

Make sure that your tests are testing one thing!

This will fail if you changed your conversion functions. Maybe you want to add 1 decimal place in the future.

it('should append a % to the converted value', function() {
      expect(filter(37, 'F', true)).toEqual('99\u00B0');
});

Change to a matcher that looks for the degrees symbol

it('should append a degrees symbol to the converted value', function() {
     expect(filter(37, 'F', true)).toContain('\u00B0');
});

Lastly, the only acceptable values for the scale are F and C, should this be case sensitive?

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.