From 071e00774e9e53357b17768195d8f27e885fca15 Mon Sep 17 00:00:00 2001 From: Nik Clayton Date: Sat, 10 Jun 2023 16:29:26 +0200 Subject: [PATCH] Replace shortNumber() with formatNumber() (#3519) formatNumber() was existing code to show numbers with suffixes like K, M, etc, so re-use that code and delete shortNumber(). Update the tests to (a) test formatNumber(), and (b) be parameterised. --- .../tusky/adapter/StatusBaseViewHolder.java | 2 +- .../tusky/adapter/StatusViewHolder.java | 4 +- .../tusky/adapter/TrendingTagViewHolder.kt | 25 +----- .../keylesspalace/tusky/util/NumberUtils.kt | 36 ++++---- .../tusky/util/NumberUtilsTest.kt | 89 ++++++++++++------- 5 files changed, 78 insertions(+), 78 deletions(-) diff --git a/app/src/main/java/com/keylesspalace/tusky/adapter/StatusBaseViewHolder.java b/app/src/main/java/com/keylesspalace/tusky/adapter/StatusBaseViewHolder.java index 3bc1a26f..de4d2380 100644 --- a/app/src/main/java/com/keylesspalace/tusky/adapter/StatusBaseViewHolder.java +++ b/app/src/main/java/com/keylesspalace/tusky/adapter/StatusBaseViewHolder.java @@ -397,7 +397,7 @@ public abstract class StatusBaseViewHolder extends RecyclerView.ViewHolder { if (replyCountLabel == null) return; if (fullStats) { - replyCountLabel.setText(NumberUtils.shortNumber(repliesCount)); + replyCountLabel.setText(NumberUtils.formatNumber(repliesCount, 1000)); return; } diff --git a/app/src/main/java/com/keylesspalace/tusky/adapter/StatusViewHolder.java b/app/src/main/java/com/keylesspalace/tusky/adapter/StatusViewHolder.java index 0763b96a..304cf93a 100644 --- a/app/src/main/java/com/keylesspalace/tusky/adapter/StatusViewHolder.java +++ b/app/src/main/java/com/keylesspalace/tusky/adapter/StatusViewHolder.java @@ -114,11 +114,11 @@ public class StatusViewHolder extends StatusBaseViewHolder { } protected void setReblogsCount(int reblogsCount) { - reblogsCountLabel.setText(NumberUtils.shortNumber(reblogsCount)); + reblogsCountLabel.setText(NumberUtils.formatNumber(reblogsCount, 1000)); } protected void setFavouritedCount(int favouritedCount) { - favouritedCountLabel.setText(NumberUtils.shortNumber(favouritedCount)); + favouritedCountLabel.setText(NumberUtils.formatNumber(favouritedCount, 1000)); } protected void hideStatusInfo() { diff --git a/app/src/main/java/com/keylesspalace/tusky/adapter/TrendingTagViewHolder.kt b/app/src/main/java/com/keylesspalace/tusky/adapter/TrendingTagViewHolder.kt index f852c46b..71a83c98 100644 --- a/app/src/main/java/com/keylesspalace/tusky/adapter/TrendingTagViewHolder.kt +++ b/app/src/main/java/com/keylesspalace/tusky/adapter/TrendingTagViewHolder.kt @@ -20,10 +20,8 @@ import com.keylesspalace.tusky.R import com.keylesspalace.tusky.databinding.ItemTrendingCellBinding import com.keylesspalace.tusky.entity.TrendingTagHistory import com.keylesspalace.tusky.interfaces.LinkListener +import com.keylesspalace.tusky.util.formatNumber import com.keylesspalace.tusky.viewdata.TrendingViewData -import java.text.NumberFormat -import kotlin.math.ln -import kotlin.math.pow class TrendingTagViewHolder( private val binding: ItemTrendingCellBinding @@ -70,25 +68,4 @@ class TrendingTagViewHolder( itemView.contentDescription = itemView.context.getString(R.string.accessibility_talking_about_tag, totalAccounts, tag) } - - companion object { - private val numberFormatter: NumberFormat = NumberFormat.getInstance() - private val ln_1k = ln(1000.0) - - /** - * Format numbers according to the current locale. Numbers < min have - * separators (',', '.', etc) inserted according to the locale. - * - * Numbers > min are scaled down to that by multiples of 1,000, and - * a suffix appropriate to the scaling is appended. - */ - private fun formatNumber(num: Long, min: Int = 100000): String { - if (num < min) return numberFormatter.format(num) - - val exp = (ln(num.toDouble()) / ln_1k).toInt() - - // TODO: is the choice of suffixes here locale-agnostic? - return String.format("%.1f %c", num / 1000.0.pow(exp.toDouble()), "KMGTPE"[exp - 1]) - } - } } diff --git a/app/src/main/java/com/keylesspalace/tusky/util/NumberUtils.kt b/app/src/main/java/com/keylesspalace/tusky/util/NumberUtils.kt index 6adb4d80..29a2ec67 100644 --- a/app/src/main/java/com/keylesspalace/tusky/util/NumberUtils.kt +++ b/app/src/main/java/com/keylesspalace/tusky/util/NumberUtils.kt @@ -2,25 +2,27 @@ package com.keylesspalace.tusky.util -import java.text.DecimalFormat +import java.text.NumberFormat import kotlin.math.abs -import kotlin.math.floor -import kotlin.math.log10 +import kotlin.math.ln import kotlin.math.pow -import kotlin.math.sign -val shortLetters = arrayOf(' ', 'K', 'M', 'B', 'T', 'P', 'E') +private val numberFormatter: NumberFormat = NumberFormat.getInstance() +private val ln_1k = ln(1000.0) -fun shortNumber(number: Number): String { - val numberAsDouble = number.toDouble() - val nonNegativeValue = abs(numberAsDouble) - var sign = "" - if (numberAsDouble.sign < 0) { sign = "-" } - val value = floor(log10(nonNegativeValue)).toInt() - val base = value / 3 - if (value >= 3 && base < shortLetters.size) { - return DecimalFormat("$sign#0.0").format(nonNegativeValue / 10.0.pow((base * 3).toDouble())) + shortLetters[base] - } else { - return DecimalFormat("$sign#,##0").format(nonNegativeValue) - } +/** + * Format numbers according to the current locale. Numbers < min have + * separators (',', '.', etc) inserted according to the locale. + * + * Numbers >= min are scaled down to that by multiples of 1,000, and + * a suffix appropriate to the scaling is appended. + */ +fun formatNumber(num: Long, min: Int = 100000): String { + val absNum = abs(num) + if (absNum < min) return numberFormatter.format(num) + + val exp = (ln(absNum.toDouble()) / ln_1k).toInt() + + // Suffixes here are locale-agnostic + return String.format("%.1f%c", num / 1000.0.pow(exp.toDouble()), "KMGTPE"[exp - 1]) } diff --git a/app/src/test/java/com/keylesspalace/tusky/util/NumberUtilsTest.kt b/app/src/test/java/com/keylesspalace/tusky/util/NumberUtilsTest.kt index 3654176b..25ac69ee 100644 --- a/app/src/test/java/com/keylesspalace/tusky/util/NumberUtilsTest.kt +++ b/app/src/test/java/com/keylesspalace/tusky/util/NumberUtilsTest.kt @@ -1,49 +1,70 @@ package com.keylesspalace.tusky.util +import org.junit.AfterClass import org.junit.Assert +import org.junit.BeforeClass import org.junit.Test +import org.junit.runner.RunWith +import org.junit.runners.Parameterized +import java.util.Locale import kotlin.math.pow -class NumberUtilsTest { +@RunWith(Parameterized::class) +class NumberUtilsTest(private val input: Long, private val want: String) { + companion object { + /** Default locale before this test started */ + private lateinit var locale: Locale - @Test - fun zeroShouldBeFormattedAsZero() { - val shortNumber = shortNumber(0) - Assert.assertEquals("0", shortNumber) - } + /** + * Ensure the Locale is ENGLISH so that tests against literal strings like + * "1.0M" later, even if the test host's locale is e.g. GERMAN which would + * normally report "1,0M". + */ + @BeforeClass + @JvmStatic + fun beforeClass() { + locale = Locale.getDefault() + Locale.setDefault(Locale.ENGLISH) + } - @Test - fun negativeValueShouldBeFormattedToNegativeValue() { - val shortNumber = shortNumber(-1) - Assert.assertEquals("-1", shortNumber) - } + @AfterClass + @JvmStatic + fun afterClass() { + Locale.setDefault(locale) + } - @Test - fun positiveValueShouldBeFormattedToPositiveValue() { - val shortNumber = shortNumber(1) - Assert.assertEquals("1", shortNumber) - } - - @Test - fun bigNumbersShouldBeShortened() { - var shortNumber = 1L - Assert.assertEquals("1", shortNumber(shortNumber)) - for (i in shortLetters.indices) { - if (i == 0) { - continue - } - shortNumber = 1000.0.pow(i.toDouble()).toLong() - Assert.assertEquals("1.0" + shortLetters[i], shortNumber(shortNumber)) + @Parameterized.Parameters(name = "formatNumber_{0}") + @JvmStatic + fun data(): Iterable { + return listOf( + arrayOf(0, "0"), + arrayOf(1, "1"), + arrayOf(-1, "-1"), + arrayOf(999, "999"), + arrayOf(1000, "1.0K"), + arrayOf(1500, "1.5K"), + arrayOf(-1500, "-1.5K"), + arrayOf(1000.0.pow(2).toLong(), "1.0M"), + arrayOf(1000.0.pow(3).toLong(), "1.0G"), + arrayOf(1000.0.pow(4).toLong(), "1.0T"), + arrayOf(1000.0.pow(5).toLong(), "1.0P"), + arrayOf(1000.0.pow(6).toLong(), "1.0E"), + arrayOf(3, "3"), + arrayOf(35, "35"), + arrayOf(350, "350"), + arrayOf(3500, "3.5K"), + arrayOf(-3500, "-3.5K"), + arrayOf(3500 * 1000, "3.5M"), + arrayOf(3500 * 1000.0.pow(2).toLong(), "3.5G"), + arrayOf(3500 * 1000.0.pow(3).toLong(), "3.5T"), + arrayOf(3500 * 1000.0.pow(4).toLong(), "3.5P"), + arrayOf(3500 * 1000.0.pow(5).toLong(), "3.5E") + ) } } @Test - fun roundingForNegativeAndPositiveValuesShouldBeTheSame() { - var value = 3492 - Assert.assertEquals("-3.5K", shortNumber(-value)) - Assert.assertEquals("3.5K", shortNumber(value)) - value = 1501 - Assert.assertEquals("-1.5K", shortNumber(-value)) - Assert.assertEquals("1.5K", shortNumber(value)) + fun test() { + Assert.assertEquals(want, formatNumber(input, 1000)) } }