New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: alternative to of:formatBytes() with a base of 1000 #204

Closed
jepsar opened this Issue Feb 1, 2016 · 9 comments

Comments

Projects
None yet
2 participants
@jepsar

jepsar commented Feb 1, 2016

A lot of end users don't know about or understand the binary notation

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Feb 2, 2016

Member

Coincidentally, for a slight different functional requirement at job I've added of:formatThousands().

Only difference with your requirement is that it prints the unit in all-lowercase and doesn't suffix any unit type such as "b". You could easily do that yourself.

File size: #{of:formatThousands(file.size)}b

Is this acceptable?

Member

BalusC commented Feb 2, 2016

Coincidentally, for a slight different functional requirement at job I've added of:formatThousands().

Only difference with your requirement is that it prints the unit in all-lowercase and doesn't suffix any unit type such as "b". You could easily do that yourself.

File size: #{of:formatThousands(file.size)}b

Is this acceptable?

@jepsar

This comment has been minimized.

Show comment
Hide comment
@jepsar

jepsar Feb 2, 2016

That would be really usable! I could totally live with adding the "b". I would prefer the units to be like "kMGTPE" though.

jepsar commented Feb 2, 2016

That would be really usable! I could totally live with adding the "b". I would prefer the units to be like "kMGTPE" though.

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Feb 2, 2016

Member

I myself wasn't exactly sure about that though. I looked at sites like Stack Overflow, Facebook and Google+ and they use lowercase for at least "k" and "m".

In hindsight, using "kMGTPE" is actually better. I will change that.

Member

BalusC commented Feb 2, 2016

I myself wasn't exactly sure about that though. I looked at sites like Stack Overflow, Facebook and Google+ and they use lowercase for at least "k" and "m".

In hindsight, using "kMGTPE" is actually better. I will change that.

@BalusC BalusC closed this in 37c4206 Feb 2, 2016

@jepsar

This comment has been minimized.

Show comment
Hide comment
@jepsar

jepsar Feb 2, 2016

Thanks! The Java doc and taglib xml still have lower case "m" in the examples: "<li>9994999 will appear as 9.99m"...

jepsar commented Feb 2, 2016

Thanks! The Java doc and taglib xml still have lower case "m" in the examples: "<li>9994999 will appear as 9.99m"...

BalusC added a commit that referenced this issue Feb 2, 2016

@jepsar

This comment has been minimized.

Show comment
Hide comment
@jepsar

jepsar Feb 3, 2016

Just one small detail. formatThousands suffixes the unit prefix without a space. That's OK when you just want to format a plain number like StackOverflow does, and that's how the method was intended in the first place. However, when you want to add a unit, a space commonly used before the unit and it's prefix (for example 1.58 kW).

Maybe it's an idea to implement it like this:

     */
    public static String formatThousands(Number number) {
        return formatThousandsUnit(number, null);
    }


    /**
     * Format the given number to nearest 10<sup>n</sup> (rounded to thousands), suffixed with a space, the metric unit
     * prefix (k, M, G, T, P or E) and the given unit, rounding half up with a precision of 3 digits, whereafter trailing
     * zeroes in fraction part are stripped.
     * @param number The number to be formatted.
     * @param unit The unit used in format.
     * @return The formatted number.
     * @see #formatThousands(java.lang.Number)
     * @since 2.3
     */
    public static String formatThousandsUnit(Number number, String unit) {
        if (number == null) {
            return null;
        }

        String separator = unit == null ? "" : " ";
        String unitString = unit == null ? "" : unit;
        BigDecimal decimal;

        try {
            decimal = new BigDecimal(number.toString());
        }
        catch (NumberFormatException e) {
            return null;
        }

        if (decimal.longValue() < NUMBER_1K) {
            return number.toString() + separator + unitString;
        }

        int exp = (int) (Math.log(decimal.longValue()) / Math.log(NUMBER_1K));
        BigDecimal divided = decimal.divide(BigDecimal.valueOf(Math.pow(NUMBER_1K, exp)));
        int maxfractions = 3 - String.valueOf(divided.longValue()).length();
        return String.format(getLocale(), "%." + maxfractions + "f", divided).replaceAll("\\D?0+$", "") +
               separator + "kMGTPE".charAt(exp - 1) + unitString;
    }

jepsar commented Feb 3, 2016

Just one small detail. formatThousands suffixes the unit prefix without a space. That's OK when you just want to format a plain number like StackOverflow does, and that's how the method was intended in the first place. However, when you want to add a unit, a space commonly used before the unit and it's prefix (for example 1.58 kW).

Maybe it's an idea to implement it like this:

     */
    public static String formatThousands(Number number) {
        return formatThousandsUnit(number, null);
    }


    /**
     * Format the given number to nearest 10<sup>n</sup> (rounded to thousands), suffixed with a space, the metric unit
     * prefix (k, M, G, T, P or E) and the given unit, rounding half up with a precision of 3 digits, whereafter trailing
     * zeroes in fraction part are stripped.
     * @param number The number to be formatted.
     * @param unit The unit used in format.
     * @return The formatted number.
     * @see #formatThousands(java.lang.Number)
     * @since 2.3
     */
    public static String formatThousandsUnit(Number number, String unit) {
        if (number == null) {
            return null;
        }

        String separator = unit == null ? "" : " ";
        String unitString = unit == null ? "" : unit;
        BigDecimal decimal;

        try {
            decimal = new BigDecimal(number.toString());
        }
        catch (NumberFormatException e) {
            return null;
        }

        if (decimal.longValue() < NUMBER_1K) {
            return number.toString() + separator + unitString;
        }

        int exp = (int) (Math.log(decimal.longValue()) / Math.log(NUMBER_1K));
        BigDecimal divided = decimal.divide(BigDecimal.valueOf(Math.pow(NUMBER_1K, exp)));
        int maxfractions = 3 - String.valueOf(divided.longValue()).length();
        return String.format(getLocale(), "%." + maxfractions + "f", divided).replaceAll("\\D?0+$", "") +
               separator + "kMGTPE".charAt(exp - 1) + unitString;
    }
@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Feb 6, 2016

Member

Makes sense.

Member

BalusC commented Feb 6, 2016

Makes sense.

BalusC added a commit that referenced this issue Feb 6, 2016

@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Feb 6, 2016

Member

It's available in today's snapshot.

Still wondering if I should switch back to all-lowercase unit suffix in of:formatThousands() (i.e. when unit is specified as null (not as empty string). I think I will do, but let's medidate about that this week.

Member

BalusC commented Feb 6, 2016

It's available in today's snapshot.

Still wondering if I should switch back to all-lowercase unit suffix in of:formatThousands() (i.e. when unit is specified as null (not as empty string). I think I will do, but let's medidate about that this week.

@jepsar

This comment has been minimized.

Show comment
Hide comment
@jepsar

jepsar Feb 7, 2016

I would stick with the SI metric prefix symbols. If you want to use lower case in your presentation layer the preferred way to go is using a rule in your style sheet. If you aren't outputting HTML you can also lower case the output in EL.

If you want to reduce duplicate code, you could add method to format numbers with a variable base:

public static String formatBytes(Long bytes) {
  return formatBaseUnit(bytes, BYTES_1K, "B");
}

public static String formatThousandsUnit(Number number, String unit) {
  return formatBaseUnit(number, NUMBER_1K, unit);
}

private static formatBaseUnit(Number number, int base, String unit) {
  // ..
}

jepsar commented Feb 7, 2016

I would stick with the SI metric prefix symbols. If you want to use lower case in your presentation layer the preferred way to go is using a rule in your style sheet. If you aren't outputting HTML you can also lower case the output in EL.

If you want to reduce duplicate code, you could add method to format numbers with a variable base:

public static String formatBytes(Long bytes) {
  return formatBaseUnit(bytes, BYTES_1K, "B");
}

public static String formatThousandsUnit(Number number, String unit) {
  return formatBaseUnit(number, NUMBER_1K, unit);
}

private static formatBaseUnit(Number number, int base, String unit) {
  // ..
}
@BalusC

This comment has been minimized.

Show comment
Hide comment
@BalusC

BalusC Feb 7, 2016

Member

Of course, CSS. Thank you!

Refactoring was planned but I somehow overlooked it due to all the fuzz. Thanks for heads up!

Member

BalusC commented Feb 7, 2016

Of course, CSS. Thank you!

Refactoring was planned but I somehow overlooked it due to all the fuzz. Thanks for heads up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment