shortFmt and floats

[Rodeoclash]

shortFmt doesn't seem to like floats much, would it be worth casting them to an int inside the filter? I'm currently doing outside.

[Rodeoclash]

Also doesn't like negative numbers

[a8m]

Cast to int sounds like a great solution. Thanks @Rodeoclash , feel free to PR.

[Rodeoclash]

Okie dokie, I'll send through in about 14 hours :)

[SoundLogic]

@Rodeoclash this function has served me pretty well so far - will need some refactoring to work with this codebase, but hopefully is helpful:

        function makeFriendlyNumber(value, fixedDecimals, prefix, formatFunction) {
            if(!prefix) { prefix = ''; }
            value = parseFloat(value);
            if (value == 0) { return String(prefix) + '0'; }
            var k = 1000;
            var sizes = ['','K', 'M', 'B', 'T', 'Q'];
            var i = Math.floor(Math.log(value) / Math.log(k));
            var rawResult = (value / Math.pow(k, i)).toFixed(fixedDecimals);
            if(!formatFunction){
                return String(prefix) + rawResult  + ' ' + sizes[i];
            } else {
                var rawNumericResult = parseFloat(rawResult );
                return String(prefix) + formatFunction(rawNumericResult ) + ' ' + sizes[i];
            }
        }

the "formatFunction" is just there in case the caller wanted to do something like use an angular $filter to convert the number to a localized currency format or what have you

[Rodeoclash]

I realised that it was handling floats correctly, I was just trying to pass it strings. That said, what do you think about a filter for parsing strings to ints / floats? I'm currently doing this by $scope.parseInt = parseInt but it would be better suited to a filter.

Do you think that belongs in angular-filter? I can add if you want.

[SoundLogic]

@a8m what are your thoughts on this? should strings be allowed? I don't see any reason to limit this other than, perhaps, consistency? happy to contribute! p.s. I like that you used memoization :smiley:

[SoundLogic]

if you want to allow strings, my proposed function would be:

    function(value, decimals) {
        if(!value) { return 'NaN'; }
        if(!decimals) { decimals = 0; }
        value = parseFloat(value);
        if (value == 0) { return '0'; }
        var k = 1000, sizes = ['','K', 'M', 'B', 'T', 'Q'];
        // i = nth exponent of k by way of the change of base formula for logs
        // then taking the floor of that to get only the integer value of the number
        var i = $math.floor($math.log(value) / $math.log(k));
        return (value / $math.pow(k, i)).toFixed(fixedDecimals) + ' ' + sizes[i];
    }

[a8m]

LGTM to support numbers as a strings, and we can go this way to test it..

if(isString(input)) {
  var num = Number(input);
  return num != num  // isNaN
    ? input
    : // keep going...
}

About the implementation, it's your choice guys, just stay in the same convention and don't forget about the tests. :) Thanks.

[SoundLogic]

yes.. that makes more sense

so something like...

        function tryConvertToNumber(input) {
            // this is not a trash bin
            if (!input) {
                return input;
            }

            if (isString(input)) {
                var num = Number(input);
                return num != num // isNaN
                    ? input
                    : "NaN";
            } else {
                return input;
            }
        }

        function(value, decimals) {
            value = tryConvertToNumber(value);
            decimals = tryConvertToNumber(decimals);
            if(NaN(value)){ return 'NaN'; }
            if(!decimals || NaN(decimals)) { decimals = 0; }
            value = parseFloat(value);
            if (value == 0) { return '0'; }
            var k = 1000, sizes = ['','K', 'M', 'B', 'T', 'Q'];
            var i = $math.floor($math.log(value) / $math.log(k));
            return (value / $math.pow(k, i)).toFixed(decimals) + ' ' + sizes[i];
        }

[Rodeoclash]

Here's the pull request for handling the negative numbers:

https://github.com/a8m/angular-filter/pull/76

I think shortFmt should throw an exception if you pass it a string rather than try to handle it. You're mixing concerns.

I was thinking that you could add two additional filters, toInt andtoFloat. Usage with shortFmt would be something like:

{{myStringNumber | toInt | shortFmt:2}}

Thoughts?

[SoundLogic]

I am finding a hard time finding a downside to supporting strings here. I understand where you're coming from theoretically speaking and completely respect that. Considering, however, that this is a display value and we are not actually changing the underlying model value, I'm not sure it would hurt to make the method a bit more robust and keep the API simple (counter-examples welcome). If someone felt this posed a performance issue, we could add an optional flag to allow strings (which would default to true if not otherwise declared). Thoughts?

[Rodeoclash]

Keep in mind that theirs at least two other filters that would need to implement string handling like this (btyeFmt and kbFmt) so you're going to have to share your tryConvertToNumber function between them.

I'm just not a fan of handling what I think is incorrect input. I don't think it should fail silently like it does now (and should use the $log service to notify the user) but I don't think we should try to anticipate what the user is trying to do. If we do handle strings, we need to implement in at least btyeFmt and kbFmt and what about functions that take string input, should we cast numbers into strings for the user as well? It all starts to get a bit magic.

@a8m any thoughts on this? I'll defer to how you want to implement as this is your rodeo after all :)

[a8m]

Filters mostly used in the view, IMO we can support strings. why ? like @SoundLogic sayed:

this is a display value and we are not actually changing the underlying model value

Also, $parse do it too. e.g: {{ '8' * 2 }} returns 16 even this expression {{ '8' * '2' }} returns 16.

So, for filters that take number type as an input, we can try to cast it, test if it's NaN, etc... I'm not fan of logging things in the client. and if so, at least it would be turned off by default.

Let me know what do you think guys. cheers!

[Rodeoclash]

Ok, sounds good. I'm pretty busy again now, @SoundLogic your stuff looks good though, you should pull request it.

[SoundLogic]

Will do - will try to get that change in today, but next two days are very busy so might not be until Wednesday

[SoundLogic]

@a8m do you want me to extend this tryConvertToNumber to all filter methods under _filter/math? Slightly revised function below:

        function tryConvertToNumber(input) {
            // this is not a trash bin
            if (!input) {
                return input;
            }

            if (isString(input)) {
                var num = Number(input);
                return num != num // isNaN
                    ? input
                    : "NaN";
            } else {
                return parseFloat(value);
            }
        }

        function(value, decimals) {
            value = tryConvertToNumber(value);
            decimals = tryConvertToNumber(decimals);
            if(NaN(value)){ return 'NaN'; }
            if(!decimals || NaN(decimals)) { decimals = 0; }
            if (value == 0) { return '0'; }
            var k = 1000, sizes = ['','K', 'M', 'B', 'T', 'Q'];
            var i = $math.floor($math.log(value) / $math.log(k));
            return (value / $math.pow(k, i)).toFixed(decimals) + ' ' + sizes[i];
        }

[SoundLogic]

fell a bit behind on this ... end of sprint / deployment week ... going to try to get this done now

[a8m]

Sorry about the delay @SoundLogic let me know what do you think about this design ?

// create some filter factory
$math.filterFacotry(fn) {
  return function(input) {
    input = Number(input)
    return input != input
      ? input // or maybe NaN
      : fn.apply(this, arguments)
  }
}
// and then, for example
angular.module('a8m.math.shortFmt', ['a8m.math'])
  .filter('shortFmt', ['$math', function ($math) {
    return $math.filterFactory(function(number, decimal) {
      // ...
    });
  }]);

Thanks.

[SoundLogic]

I like that approach, but do we want to support all object types Number() accepts? If someone accidentally uses a Date object they could end up with some unexpected results.. thoughts?

alsoI will incorporate this change into my fix: https://github.com/a8m/angular-filter/pull/76

[a8m]

I think we should align with $parse take a look please: example thanks.

[SoundLogic]

fair point

[SoundLogic]

okay so I've made the changes and updated the tests, but realized that toFixed isn't the same as the convertToDecimal method - I think it may be confusing referring to the decimal parameter by that name rather than as "precision", but I personally would also prefer to use a fixed number of decimals over precision (or at least have an option for this) - what are your thoughts?

also, do we want to provide an option to round up or down on the final digit?

[JoshuaHitchins]

Any progress on this? Does this need to be looked at?