refactor: Values pt. 5 - use Values::Speed in Qt client (#6248)

libtransmission/values.h

@@ -6,6 +6,7 @@
 #pragma once
 
 #include <array>
+#include <cmath> // for std::fabs(), std::floor()
 #include <cstdint> // for uint64_t
 #include <string>
 #include <string_view>
@@ -116,12 +117,6 @@ public:
     {
     }
 
-    constexpr auto& operator+=(Value const& that) noexcept
-    {
-        base_quantity_ += that.base_quantity_;
-        return *this;
-    }
-
     [[nodiscard]] constexpr auto base_quantity() const noexcept
     {
         return base_quantity_;
@@ -132,6 +127,12 @@ public:
         return base_quantity_ / (1.0 * units_.multiplier(tgt));
     }
 
+    constexpr auto& operator+=(Value const& that) noexcept
+    {
+        base_quantity_ += that.base_quantity_;
+        return *this;
+    }
+
     [[nodiscard]] constexpr auto operator+(Value const& that) noexcept
     {
         auto ret = *this;
@@ -195,18 +196,14 @@ public:
     std::string_view to_string(char* buf, size_t buflen) const noexcept
     {
         auto idx = size_t{ 0 };
-
-        if (base_quantity_ < 1000) // 0 to 999
-        {
-            *fmt::format_to_n(buf, buflen - 1, "{:d} {:s}", base_quantity_, units_.display_name(idx)).out = '\0';
-            return buf;
-        }
-
         auto val = 1.0 * base_quantity_;
         for (;;)
         {
-            ++idx;
-            val /= units_.base();
+            if (std::fabs(val - std::floor(val)) < 0.001 && (val < 999.5 || std::empty(units_.display_name(idx + 1))))
+            {
+                *fmt::format_to_n(buf, buflen - 1, "{:.0Lf} {:s}", val, units_.display_name(idx)).out = '\0';
+                return buf;
+            }
 
             if (val < 99.995) // 0.98 to 99.99
             {
@@ -219,6 +216,9 @@ public:
                 *fmt::format_to_n(buf, buflen - 1, "{:.1Lf} {:s}", val, units_.display_name(idx)).out = '\0';
                 return buf;
             }
+
+            val /= units_.base();
+            ++idx;
         }
     }
 

qt/DetailsDialog.cc

@@ -1039,8 +1039,8 @@ void DetailsDialog::refreshUI()
 
         setIfIdle(ui_.bandwidthPriorityCombo, i);
 
-        setIfIdle(ui_.singleDownSpin, static_cast<int>(baseline.downloadLimit().getKBps()));
-        setIfIdle(ui_.singleUpSpin, static_cast<int>(baseline.uploadLimit().getKBps()));
+        setIfIdle(ui_.singleDownSpin, static_cast<int>(baseline.downloadLimit().count(Speed::Units::KByps)));
+        setIfIdle(ui_.singleUpSpin, static_cast<int>(baseline.uploadLimit().count(Speed::Units::KByps)));
         setIfIdle(ui_.peerLimitSpin, baseline.peerLimit());
     }
 
@@ -1201,8 +1201,8 @@ void DetailsDialog::refreshUI()
                 code_tip.resize(code_tip.size() - 1); // eat the trailing linefeed
             }
 
-            item->setText(COL_UP, peer.rate_to_peer.isZero() ? QString{} : fmt.speedToString(peer.rate_to_peer));
-            item->setText(COL_DOWN, peer.rate_to_client.isZero() ? QString{} : fmt.speedToString(peer.rate_to_client));
+            item->setText(COL_UP, peer.rate_to_peer.is_zero() ? QString{} : peer.rate_to_peer.to_qstring());
+            item->setText(COL_DOWN, peer.rate_to_client.is_zero() ? QString{} : peer.rate_to_client.to_qstring());
             item->setText(
                 COL_PERCENT,
                 peer.progress > 0 ? QStringLiteral("%1%").arg(static_cast<int>(peer.progress * 100.0)) : QString{});

qt/Formatter.cc

@@ -5,12 +5,15 @@
 
 #include <libtransmission/transmission.h>
 #include <libtransmission/utils.h> // tr_formatter
+#include <libtransmission/values.h> // tr_formatter
 
 #include "Formatter.h"
 #include "Speed.h"
 
 #include <algorithm>
 
+using namespace std::literals;
+
 Formatter& Formatter::get()
 {
     // NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
@@ -25,13 +28,12 @@ Formatter::Formatter()
           { tr("B"), tr("KiB"), tr("MiB"), tr("GiB"), tr("TiB") } // MEM
       } }
 {
+    namespace Values = libtransmission::Values;
+
     auto const& speed = UnitStrings[SPEED];
-    tr_formatter_speed_init(
-        SpeedBase,
-        speed[KB].toUtf8().constData(),
-        speed[MB].toUtf8().constData(),
-        speed[GB].toUtf8().constData(),
-        speed[TB].toUtf8().constData());
+    Values::Config::Speed = { Values::Config::Base::Kilo, "B"sv,
+                              speed[KB].toStdString(),    speed[MB].toStdString(),
+                              speed[GB].toStdString(),    speed[TB].toStdString() };
 
     auto const& size = UnitStrings[SIZE];
     tr_formatter_size_init(
@@ -117,17 +119,3 @@ QString Formatter::timeToString(int seconds) const
 
     return tr("%Ln day(s)", nullptr, days);
 }
-
-/***
-****
-***/
-
-double Speed::getKBps() const
-{
-    return getBps() / static_cast<double>(Formatter::SpeedBase);
-}
-
-Speed Speed::fromKBps(double KBps)
-{
-    return Speed{ static_cast<int>(KBps * Formatter::SpeedBase) };
-}

qt/Formatter.h

@@ -40,7 +40,6 @@ public:
         NUM_TYPES
     };
 
-    static constexpr int SpeedBase = 1000;
     static constexpr int SizeBase = 1000;
     static constexpr int MemBase = 1024;
 
@@ -52,25 +51,6 @@ public:
     [[nodiscard]] QString timeToString(int seconds) const;
     [[nodiscard]] QString unitStr(Type t, Size s) const;
 
-    [[nodiscard]] auto speedToString(Speed const& speed) const
-    {
-        return QString::fromStdString(tr_formatter_speed_KBps(speed.getKBps()));
-    }
-
-    [[nodiscard]] auto uploadSpeedToString(Speed const& upload_speed) const
-    {
-        static auto constexpr UploadSymbol = QChar{ 0x25B4 };
-
-        return tr("%1 %2").arg(speedToString(upload_speed)).arg(UploadSymbol);
-    }
-
-    [[nodiscard]] auto downloadSpeedToString(Speed const& download_speed) const
-    {
-        static auto constexpr DownloadSymbol = QChar{ 0x25BE };
-
-        return tr("%1 %2").arg(speedToString(download_speed)).arg(DownloadSymbol);
-    }
-
     [[nodiscard]] auto percentToString(double x) const
     {
         return QString::fromStdString(tr_strpercent(x));

qt/MainWindow.cc

@@ -391,7 +391,7 @@ void MainWindow::initStatusBar()
     ui_.optionsButton->setMenu(createOptionsMenu());
 
     int const minimum_speed_width = ui_.downloadSpeedLabel->fontMetrics()
-                                        .size(0, Formatter::get().uploadSpeedToString(Speed::fromKBps(999.99)))
+                                        .size(0, Speed{ 999.99, Speed::Units::KByps }.to_qstring())
                                         .width();
     ui_.downloadSpeedLabel->setMinimumWidth(minimum_speed_width);
     ui_.uploadSpeedLabel->setMinimumWidth(minimum_speed_width);
@@ -416,7 +416,7 @@ QMenu* MainWindow::createOptionsMenu()
         action_group->addAction(off_action);
         connect(off_action, &QAction::triggered, this, qOverload<bool>(&MainWindow::onSetPrefs));
 
-        on_action = menu->addAction(tr("Limited at %1").arg(Formatter::get().speedToString(Speed::fromKBps(current_value))));
+        on_action = menu->addAction(tr("Limited at %1").arg(Speed{ current_value, Speed::Units::KByps }.to_qstring()));
         on_action->setCheckable(true);
         on_action->setProperty(PrefVariantsKey, QVariantList{ pref, current_value, enabled_pref, true });
         action_group->addAction(on_action);
@@ -424,10 +424,10 @@ QMenu* MainWindow::createOptionsMenu()
 
         menu->addSeparator();
 
-        for (auto const kps : { 50, 100, 250, 500, 1000, 2500, 5000, 10000 })
+        for (auto const kbyps : { 50, 100, 250, 500, 1000, 2500, 5000, 10000 })
         {
-            auto* const action = menu->addAction(Formatter::get().speedToString(Speed::fromKBps(kps)));
-            action->setProperty(PrefVariantsKey, QVariantList{ pref, kps, enabled_pref, true });
+            auto* const action = menu->addAction(Speed{ kbyps, Speed::Units::KByps }.to_qstring());
+            action->setProperty(PrefVariantsKey, QVariantList{ pref, kbyps, enabled_pref, true });
             connect(action, &QAction::triggered, this, qOverload<>(&MainWindow::onSetPrefs));
         }
     };
@@ -812,12 +812,11 @@ void MainWindow::refreshTrayIcon(TransferStats const& stats)
     }
     else if (stats.peers_sending != 0)
     {
-        tip = Formatter::get().downloadSpeedToString(stats.speed_down) + QStringLiteral("   ") +
-            Formatter::get().uploadSpeedToString(stats.speed_up);
+        tip = stats.speed_down.to_download_qstring() + QStringLiteral("   ") + stats.speed_up.to_upload_qstring();
     }
     else if (stats.peers_receiving != 0)
     {
-        tip = Formatter::get().uploadSpeedToString(stats.speed_up);
+        tip = stats.speed_up.to_upload_qstring();
     }
 
     tray_icon_.setToolTip(tip);
@@ -826,9 +825,9 @@ void MainWindow::refreshTrayIcon(TransferStats const& stats)
 void MainWindow::refreshStatusBar(TransferStats const& stats)
 {
     auto const& fmt = Formatter::get();
-    ui_.uploadSpeedLabel->setText(fmt.uploadSpeedToString(stats.speed_up));
+    ui_.uploadSpeedLabel->setText(stats.speed_up.to_upload_qstring());
     ui_.uploadSpeedLabel->setVisible(stats.peers_sending || stats.peers_receiving);
-    ui_.downloadSpeedLabel->setText(fmt.downloadSpeedToString(stats.speed_down));
+    ui_.downloadSpeedLabel->setText(stats.speed_down.to_download_qstring());
     ui_.downloadSpeedLabel->setVisible(stats.peers_sending);
 
     ui_.networkLabel->setVisible(!session_.isServer());
@@ -1172,7 +1171,7 @@ void MainWindow::refreshPref(int key)
 
     case Prefs::DSPEED:
         dlimit_on_action_->setText(
-            tr("Limited at %1").arg(Formatter::get().speedToString(Speed::fromKBps(prefs_.get<int>(key)))));
+            tr("Limited at %1").arg(Speed{ prefs_.get<unsigned int>(key), Speed::Units::KByps }.to_qstring()));
         break;
 
     case Prefs::USPEED_ENABLED:
@@ -1181,7 +1180,7 @@ void MainWindow::refreshPref(int key)
 
     case Prefs::USPEED:
         ulimit_on_action_->setText(
-            tr("Limited at %1").arg(Formatter::get().speedToString(Speed::fromKBps(prefs_.get<int>(key)))));
+            tr("Limited at %1").arg(Speed{ prefs_.get<unsigned int>(key), Speed::Units::KByps }.to_qstring()));
         break;
 
     case Prefs::RATIO_ENABLED:
@@ -1244,9 +1243,9 @@ void MainWindow::refreshPref(int key)
             ui_.altSpeedButton->setChecked(b);
             QString const fmt = b ? tr("Click to disable Temporary Speed Limits\n (%1 down, %2 up)") :
                                     tr("Click to enable Temporary Speed Limits\n (%1 down, %2 up)");
-            Speed const d = Speed::fromKBps(prefs_.getInt(Prefs::ALT_SPEED_LIMIT_DOWN));
-            Speed const u = Speed::fromKBps(prefs_.getInt(Prefs::ALT_SPEED_LIMIT_UP));
-            ui_.altSpeedButton->setToolTip(fmt.arg(Formatter::get().speedToString(d)).arg(Formatter::get().speedToString(u)));
+            auto const d = Speed{ prefs_.get<unsigned int>(Prefs::ALT_SPEED_LIMIT_DOWN), Speed::Units::KByps };
+            auto const u = Speed{ prefs_.get<unsigned int>(Prefs::ALT_SPEED_LIMIT_UP), Speed::Units::KByps };
+            ui_.altSpeedButton->setToolTip(fmt.arg(d.to_qstring()).arg(u.to_qstring()));
             break;
         }
 

qt/Speed.h

@@ -5,66 +5,54 @@
 
 #pragma once
 
-class Speed
+#include <QCoreApplication> // Q_DECLARE_TR_FUNCTIONS
+#include <QString>
+
+#include "libtransmission/values.h"
+
+class Speed : public libtransmission::Values::Speed
 {
+    Q_DECLARE_TR_FUNCTIONS(Speed)
+
 public:
     Speed() = default;
 
-    double getKBps() const;
-
-    [[nodiscard]] auto constexpr getBps() const noexcept
-    {
-        return bytes_per_second_;
-    }
-
-    [[nodiscard]] auto constexpr isZero() const noexcept
-    {
-        return bytes_per_second_ == 0;
-    }
-
-    static Speed fromKBps(double KBps);
-
-    [[nodiscard]] static constexpr Speed fromBps(int Bps) noexcept
-    {
-        return Speed{ Bps };
-    }
-
-    void constexpr setBps(int Bps) noexcept
-    {
-        bytes_per_second_ = Bps;
-    }
-
-    constexpr Speed& operator+=(Speed const& that) noexcept
-    {
-        bytes_per_second_ += that.bytes_per_second_;
-        return *this;
-    }
-
-    [[nodiscard]] auto constexpr operator+(Speed const& that) const noexcept
-    {
-        return Speed{ getBps() + that.getBps() };
-    }
-
-    [[nodiscard]] auto constexpr operator<(Speed const& that) const noexcept
-    {
-        return getBps() < that.getBps();
-    }
-
-    [[nodiscard]] auto constexpr operator==(Speed const& that) const noexcept
-    {
-        return getBps() == that.getBps();
-    }
-
-    [[nodiscard]] auto constexpr operator!=(Speed const& that) const noexcept
-    {
-        return getBps() != that.getBps();
-    }
-
-private:
-    explicit constexpr Speed(int bytes_per_second) noexcept
-        : bytes_per_second_{ bytes_per_second }
+    template<typename Number, typename std::enable_if_t<std::is_integral_v<Number>>* = nullptr>
+    constexpr Speed(Number value, Units multiple)
+        : libtransmission::Values::Speed{ value, multiple }
     {
     }
 
-    int bytes_per_second_ = {};
+    template<typename Number, typename std::enable_if_t<std::is_floating_point_v<Number>>* = nullptr>
+    Speed(Number value, Units multiple)
+        : libtransmission::Values::Speed{ value, multiple }
+    {
+    }
+
+    [[nodiscard]] auto constexpr is_zero() const noexcept
+    {
+        return base_quantity() == 0U;
+    }
+
+    [[nodiscard]] auto to_qstring() const noexcept
+    {
+        return QString::fromStdString(to_string());
+    }
+
+    [[nodiscard]] auto to_upload_qstring() const
+    {
+        static auto constexpr UploadSymbol = QChar{ 0x25B4 };
+        return tr("%1 %2").arg(to_qstring()).arg(UploadSymbol);
+    }
+
+    [[nodiscard]] auto to_download_qstring() const
+    {
+        static auto constexpr DownloadSymbol = QChar{ 0x25BE };
+        return tr("%1 %2").arg(to_qstring()).arg(DownloadSymbol);
+    }
+
+    [[nodiscard]] constexpr auto operator+(Speed const& other) const noexcept
+    {
+        return Speed{ base_quantity() + other.base_quantity(), Speed::Units::Byps };
+    }
 };

qt/Torrent.h

@@ -420,14 +420,14 @@ public:
         return sitenames_;
     }
 
-    [[nodiscard]] Speed uploadLimit() const
+    [[nodiscard]] constexpr auto uploadLimit() const
     {
-        return Speed::fromKBps(upload_limit_);
+        return Speed{ upload_limit_, Speed::Units::KByps };
     }
 
-    [[nodiscard]] Speed downloadLimit() const
+    [[nodiscard]] constexpr auto downloadLimit() const
     {
-        return Speed::fromKBps(download_limit_);
+        return Speed{ download_limit_, Speed::Units::KByps };
     }
 
     [[nodiscard]] constexpr auto uploadIsLimited() const noexcept
@@ -643,7 +643,6 @@ private:
     time_t start_date_ = {};
 
     int bandwidth_priority_ = {};
-    int download_limit_ = {};
     int error_ = {};
     int eta_ = {};
     int peer_limit_ = {};
@@ -656,10 +655,10 @@ private:
     int seed_idle_mode_ = {};
     int seed_ratio_mode_ = {};
     int status_ = {};
-    int upload_limit_ = {};
     int webseeds_sending_to_us_ = {};
 
     uint64_t desired_available_ = {};
+    uint64_t download_limit_ = {};
     uint64_t downloaded_ever_ = {};
     uint64_t failed_ever_ = {};
     uint64_t file_count_ = {};
@@ -669,6 +668,7 @@ private:
     uint64_t piece_size_ = {};
     uint64_t size_when_done_ = {};
     uint64_t total_size_ = {};
+    uint64_t upload_limit_ = {};
     uint64_t uploaded_ever_ = {};
 
     double metadata_percent_complete_ = {};

qt/TorrentDelegate.cc

@@ -278,12 +278,11 @@ QString TorrentDelegate::shortTransferString(Torrent const& tor)
 
     if (have_down)
     {
-        str = Formatter::get().downloadSpeedToString(tor.downloadSpeed()) + QStringLiteral("   ") +
-            Formatter::get().uploadSpeedToString(tor.uploadSpeed());
+        str = tor.downloadSpeed().to_download_qstring() + QStringLiteral("   ") + tor.uploadSpeed().to_upload_qstring();
     }
     else if (have_up)
     {
-        str = Formatter::get().uploadSpeedToString(tor.uploadSpeed());
+        str = tor.uploadSpeed().to_upload_qstring();
     }
 
     return str.trimmed();

qt/VariantHelpers.cc

@@ -33,8 +33,8 @@ bool change(double& setme, double const& value)
 
 bool change(Speed& setme, tr_variant const* value)
 {
-    auto const bytes_per_second = getValue<int>(value);
-    return bytes_per_second && change(setme, Speed::fromBps(*bytes_per_second));
+    auto const byps = getValue<int>(value);
+    return byps && change(setme, Speed{ *byps, Speed::Units::Byps });
 }
 
 bool change(TorrentHash& setme, tr_variant const* value)

tests/libtransmission/values-test.cc

@@ -14,24 +14,40 @@ using namespace libtransmission::Values;
 
 using ValuesTest = ::testing::Test;
 
-TEST_F(ValuesTest, value)
+TEST_F(ValuesTest, baseQuantity)
 {
     auto val = Speed{ 1, Speed::Units::MByps };
-    EXPECT_EQ("1.00 MB/s", val.to_string());
     EXPECT_EQ(1000000UL, val.base_quantity());
+}
+
+TEST_F(ValuesTest, count)
+{
+    auto const val = Speed{ 1, Speed::Units::MByps };
     EXPECT_NEAR(1000U, val.count(Speed::Units::KByps), 0.0001);
     EXPECT_NEAR(1U, val.count(Speed::Units::MByps), 0.0001);
     EXPECT_NEAR(0.001, val.count(Speed::Units::GByps), 0.0001);
+}
+
+TEST_F(ValuesTest, toString)
+{
+    auto val = Speed{ 1, Speed::Units::MByps };
+    EXPECT_EQ("1 MB/s", val.to_string());
 
     val = Speed{ 1, Speed::Units::Byps };
     EXPECT_EQ(1U, val.base_quantity());
     EXPECT_EQ("1 B/s", val.to_string());
 
     val = Speed{ 10, Speed::Units::KByps };
-    EXPECT_EQ("10.00 kB/s", val.to_string());
+    EXPECT_EQ("10 kB/s", val.to_string());
 
     val = Speed{ 999, Speed::Units::KByps };
-    EXPECT_EQ("999.0 kB/s", val.to_string());
+    EXPECT_EQ("999 kB/s", val.to_string());
+
+    val = Speed{ 99.22222, Speed::Units::KByps };
+    EXPECT_EQ("99.22 kB/s", val.to_string());
+
+    val = Speed{ 999.22222, Speed::Units::KByps };
+    EXPECT_EQ("999.2 kB/s", val.to_string());
 }
 
 TEST_F(ValuesTest, valueHonorsFormatterInit)
@@ -39,6 +55,6 @@ TEST_F(ValuesTest, valueHonorsFormatterInit)
     tr_formatter_speed_init(1024, "KayBeePerEss", "EmmBeePerEss", "GeeBeePerEss", "TeeBeePerEss");
 
     auto const val = Speed{ 1, Speed::Units::MByps };
-    EXPECT_EQ("1.00 EmmBeePerEss", val.to_string());
+    EXPECT_EQ("1 EmmBeePerEss", val.to_string());
     EXPECT_EQ(1048576U, val.base_quantity());
 }

tests/utils/assets/Inner_Sanctum_movie_archive.show

@@ -14,7 +14,7 @@ Note: retrieval usually requires a client that supports webseeding (GetRight sty
 Note: many Internet Archive torrents contain a 'pad file' directory. This directory and the files within it may be erased once retrieval completes.
 Note: the file Inner_Sanctum_movie_meta.xml contains metadata about this torrent's contents.
   Piece Count: 1090
-  Piece Size: 2.00 MiB
+  Piece Size: 2 MiB
   Total Size: 2.29 GB
   Privacy: Public torrent
 

tests/utils/assets/Thor_and_the_Amazon_Women.avi.show

@@ -9,7 +9,7 @@ GENERAL
   Created on: Mon Jan 22 00:57:20 2007
 
   Piece Count: 1493
-  Piece Size: 512.0 KiB
+  Piece Size: 512 KiB
   Total Size: 782.3 MB
   Privacy: Public torrent
 

tests/utils/assets/bitcomet.utf-8.special.show

@@ -10,7 +10,7 @@ GENERAL
 
   Source: 私たちの世界
   Piece Count: 1
-  Piece Size: 1.00 MiB
+  Piece Size: 1 MiB
   Total Size: 13 B
   Privacy: Public torrent
 

tests/utils/assets/bittorrent-v2-hybrid-test.show

@@ -10,7 +10,7 @@ GENERAL
   Created on: Wed Jun 03 08:45:06 2020
 
   Piece Count: 1715
-  Piece Size: 512.0 KiB
+  Piece Size: 512 KiB
   Total Size: 898.6 MB
   Privacy: Public torrent
 

tests/utils/assets/hybrid-single-ubuntu-20.04.3-desktop-amd64.iso.show

@@ -10,7 +10,7 @@ GENERAL
   Created on: Unknown
 
   Piece Count: 11719
-  Piece Size: 256.0 KiB
+  Piece Size: 256 KiB
   Total Size: 3.07 GB
   Privacy: Public torrent
 

tests/utils/assets/ubuntu-20.04.3-desktop-amd64.iso.show

@@ -10,7 +10,7 @@ GENERAL
 
   Comment: Ubuntu CD releases.ubuntu.com
   Piece Count: 11719
-  Piece Size: 256.0 KiB
+  Piece Size: 256 KiB
   Total Size: 3.07 GB
   Privacy: Public torrent