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.

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;
         }
 

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() {

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])
-        }
-    }
 }

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.ln
-import kotlin.math.log10
 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()
+ * Format numbers according to the current locale. Numbers < min have
-    val nonNegativeValue = abs(numberAsDouble)
+ * separators (',', '.', etc) inserted according to the locale.
-    var sign = ""
+ *
-    if (numberAsDouble.sign < 0) { sign = "-" }
+ * Numbers >= min are scaled down to that by multiples of 1,000, and
-    val value = floor(log10(nonNegativeValue)).toInt()
+ * a suffix appropriate to the scaling is appended.
-    val base = value / 3
+ */
-    if (value >= 3 && base < shortLetters.size) {
+fun formatNumber(num: Long, min: Int = 100000): String {
-        return DecimalFormat("$sign#0.0").format(nonNegativeValue / 10.0.pow((base * 3).toDouble())) + shortLetters[base]
+    val absNum = abs(num)
-    } else {
+    if (absNum < min) return numberFormatter.format(num)
-        return DecimalFormat("$sign#,##0").format(nonNegativeValue)
+
-    }
+    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])
 }

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() {
+         * Ensure the Locale is ENGLISH so that tests against literal strings like
-        val shortNumber = shortNumber(0)
+         * "1.0M" later, even if the test host's locale is e.g. GERMAN which would
-        Assert.assertEquals("0", shortNumber)
+         * normally report "1,0M".
+         */
+        @BeforeClass
+        @JvmStatic
+        fun beforeClass() {
+            locale = Locale.getDefault()
+            Locale.setDefault(Locale.ENGLISH)
         }
 
-    @Test
+        @AfterClass
-    fun negativeValueShouldBeFormattedToNegativeValue() {
+        @JvmStatic
-        val shortNumber = shortNumber(-1)
+        fun afterClass() {
-        Assert.assertEquals("-1", shortNumber)
+            Locale.setDefault(locale)
         }
 
-    @Test
+        @Parameterized.Parameters(name = "formatNumber_{0}")
-    fun positiveValueShouldBeFormattedToPositiveValue() {
+        @JvmStatic
-        val shortNumber = shortNumber(1)
+        fun data(): Iterable<Any> {
-        Assert.assertEquals("1", shortNumber)
+            return listOf(
-    }
+                arrayOf(0, "0"),
-
+                arrayOf(1, "1"),
-    @Test
+                arrayOf(-1, "-1"),
-    fun bigNumbersShouldBeShortened() {
+                arrayOf(999, "999"),
-        var shortNumber = 1L
+                arrayOf(1000, "1.0K"),
-        Assert.assertEquals("1", shortNumber(shortNumber))
+                arrayOf(1500, "1.5K"),
-        for (i in shortLetters.indices) {
+                arrayOf(-1500, "-1.5K"),
-            if (i == 0) {
+                arrayOf(1000.0.pow(2).toLong(), "1.0M"),
-                continue
+                arrayOf(1000.0.pow(3).toLong(), "1.0G"),
-            }
+                arrayOf(1000.0.pow(4).toLong(), "1.0T"),
-            shortNumber = 1000.0.pow(i.toDouble()).toLong()
+                arrayOf(1000.0.pow(5).toLong(), "1.0P"),
-            Assert.assertEquals("1.0" + shortLetters[i], shortNumber(shortNumber))
+                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() {
+    fun test() {
-        var value = 3492
+        Assert.assertEquals(want, formatNumber(input, 1000))
-        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))
     }
 }