From c2ac7fde673420555341fe79198b5900d72ab2bc Mon Sep 17 00:00:00 2001
From: Michael Carlberg <c@rlberg.se>
Date: Thu, 12 Jan 2017 20:28:44 +0100
Subject: [PATCH] refactor: Filesize strings

---
 include/modules/fs.hpp            | 14 +++---
 include/modules/memory.hpp        | 15 ++++---
 include/utils/math.hpp            |  8 ++++
 include/utils/string.hpp          | 14 +++++-
 src/modules/fs.cpp                | 34 ++++++--------
 src/modules/memory.cpp            | 75 +++++++++++--------------------
 src/utils/string.cpp              | 51 +++++++++++----------
 tests/unit_tests/utils/string.cpp | 17 +++++++
 8 files changed, 116 insertions(+), 112 deletions(-)

diff --git a/include/modules/fs.hpp b/include/modules/fs.hpp
index 7134c852..ef6ad71a 100644
--- a/include/modules/fs.hpp
+++ b/include/modules/fs.hpp
@@ -17,15 +17,13 @@ namespace modules {
     string type;
     string fsname;
 
-    unsigned long long bytes_free = 0;
-    unsigned long long bytes_used = 0;
-    unsigned long long bytes_total = 0;
+    unsigned long long bytes_free{0ULL};
+    unsigned long long bytes_used{0ULL};
+    unsigned long long bytes_avail{0ULL};
+    unsigned long long bytes_total{0ULL};
 
-    float percentage_free = 0;
-    float percentage_used = 0;
-
-    string percentage_free_s;
-    string percentage_used_s;
+    int percentage_free{0};
+    int percentage_used{0};
 
     explicit fs_mount(const string& mountpoint, bool mounted = false) : mountpoint(mountpoint), mounted(mounted) {}
   };
diff --git a/include/modules/memory.hpp b/include/modules/memory.hpp
index 3ddc27dd..f4c46547 100644
--- a/include/modules/memory.hpp
+++ b/include/modules/memory.hpp
@@ -1,7 +1,7 @@
 #pragma once
 
-#include "settings.hpp"
 #include "modules/meta/timer_module.hpp"
+#include "settings.hpp"
 
 POLYBAR_NS
 
@@ -16,14 +16,15 @@ namespace modules {
     bool build(builder* builder, const string& tag) const;
 
    private:
-    static constexpr auto TAG_LABEL = "<label>";
-    static constexpr auto TAG_BAR_USED = "<bar-used>";
-    static constexpr auto TAG_BAR_FREE = "<bar-free>";
+    static constexpr const char* TAG_LABEL{"<label>"};
+    static constexpr const char* TAG_BAR_USED{"<bar-used>"};
+    static constexpr const char* TAG_BAR_FREE{"<bar-free>"};
 
     label_t m_label;
-    progressbar_t m_bar_free;
-    map<memtype, progressbar_t> m_bars;
-    map<memtype, int> m_perc;
+    progressbar_t m_bar_memused;
+    progressbar_t m_bar_memfree;
+    int m_perc_memused{0};
+    int m_perc_memfree{0};
   };
 }
 
diff --git a/include/utils/math.hpp b/include/utils/math.hpp
index 6d2f12e8..c249748d 100644
--- a/include/utils/math.hpp
+++ b/include/utils/math.hpp
@@ -47,6 +47,14 @@ namespace math_util {
     return cap<ReturnType>(percentage, 0.0f, 100.0f);
   }
 
+  /**
+   * Calculate the percentage for a value
+   */
+  template <typename ValueType, typename ReturnType = int>
+  ReturnType percentage(ValueType value, ValueType max_value) {
+    return percentage<ValueType, ReturnType>(value, max_value - max_value, max_value);
+  }
+
   /**
    * Get value for percentage of `max_value`
    */
diff --git a/include/utils/string.hpp b/include/utils/string.hpp
index 7af4e16b..ffecf688 100644
--- a/include/utils/string.hpp
+++ b/include/utils/string.hpp
@@ -16,22 +16,32 @@ namespace string_util {
   string upper(const string& s);
   string lower(const string& s);
   bool compare(const string& s1, const string& s2);
+
   string replace(const string& haystack, const string& needle, const string& replacement, size_t start = 0,
       size_t end = string::npos);
   string replace_all(const string& haystack, const string& needle, const string& replacement, size_t start = 0,
       size_t end = string::npos);
+
   string squeeze(const string& haystack, char needle);
+
   string strip(const string& haystack, char needle);
   string strip_trailing_newline(const string& haystack);
+
   string ltrim(string&& value, const char& needle);
   string rtrim(string&& value, const char& needle);
   string trim(string&& value, const char& needle);
+
   string join(const vector<string>& strs, const string& delim);
   vector<string>& split_into(const string& s, char delim, vector<string>& container);
   vector<string> split(const string& s, char delim);
+
   size_t find_nth(const string& haystack, size_t pos, const string& needle, size_t nth);
-  string floatval(float value, int decimals = 2, bool fixed = false, const string& locale = "");
-  string filesize(unsigned long long bytes, int decimals = 2, bool fixed = false, const string& locale = "");
+
+  string floating_point(double value, size_t precision, bool fixed = false, const string& locale = "");
+  string filesize_mb(unsigned long long kbytes, size_t precision = 0, const string& locale = "");
+  string filesize_gb(unsigned long long kbytes, size_t precision = 0, const string& locale = "");
+  string filesize(unsigned long long bytes, size_t precision = 0, bool fixed = false, const string& locale = "");
+
   string from_stream(const std::basic_ostream<char>& os);
   hash_type hash(const string& src);
 }
diff --git a/src/modules/fs.cpp b/src/modules/fs.cpp
index 30e29ed4..b789b2aa 100644
--- a/src/modules/fs.cpp
+++ b/src/modules/fs.cpp
@@ -87,22 +87,11 @@ namespace modules {
         mount->mountpoint = mnt->mnt_dir;
         mount->type = mnt->mnt_type;
         mount->fsname = mnt->mnt_fsname;
-
-        auto b_total = buffer.f_bsize * buffer.f_blocks;
-        auto b_free = buffer.f_bsize * buffer.f_bfree;
-        auto b_avail = buffer.f_bsize * buffer.f_bavail;
-        auto b_used = b_total - b_avail;
-
-        mount->bytes_total = b_total;
-        mount->bytes_free = b_free;
-        mount->bytes_used = b_used;
-
-        mount->percentage_free = math_util::percentage<unsigned long, float>(b_avail, 0UL, b_total);
-        mount->percentage_used = math_util::percentage<unsigned long, float>(b_used, 0UL, b_total);
-
-        mount->percentage_free_s = string_util::floatval(mount->percentage_free, 2, m_fixed, m_bar.locale);
-        mount->percentage_used_s = string_util::floatval(mount->percentage_used, 2, m_fixed, m_bar.locale);
-
+        mount->bytes_total = buffer.f_bsize * buffer.f_blocks;
+        mount->bytes_free = buffer.f_bsize * buffer.f_bfree;
+        mount->bytes_used = mount->bytes_total - buffer.f_bsize * buffer.f_bavail;
+        mount->percentage_free = math_util::percentage<double>(mount->bytes_avail, mount->bytes_total);
+        mount->percentage_used = math_util::percentage<double>(mount->bytes_used, mount->bytes_total);
         break;
       }
     }
@@ -161,11 +150,14 @@ namespace modules {
       m_labelmounted->replace_token("%mountpoint%", mount->mountpoint);
       m_labelmounted->replace_token("%type%", mount->type);
       m_labelmounted->replace_token("%fsname%", mount->fsname);
-      m_labelmounted->replace_token("%percentage_free%", mount->percentage_free_s + "%");
-      m_labelmounted->replace_token("%percentage_used%", mount->percentage_used_s + "%");
-      m_labelmounted->replace_token("%total%", string_util::filesize(mount->bytes_total, 1, m_fixed, m_bar.locale));
-      m_labelmounted->replace_token("%free%", string_util::filesize(mount->bytes_free, 2, m_fixed, m_bar.locale));
-      m_labelmounted->replace_token("%used%", string_util::filesize(mount->bytes_used, 2, m_fixed, m_bar.locale));
+      m_labelmounted->replace_token("%percentage_free%", to_string(mount->percentage_free) + "%");
+      m_labelmounted->replace_token("%percentage_used%", to_string(mount->percentage_used) + "%");
+      m_labelmounted->replace_token(
+          "%total%", string_util::filesize(mount->bytes_total, m_fixed ? 2 : 0, m_fixed, m_bar.locale));
+      m_labelmounted->replace_token(
+          "%free%", string_util::filesize(mount->bytes_free, m_fixed ? 2 : 0, m_fixed, m_bar.locale));
+      m_labelmounted->replace_token(
+          "%used%", string_util::filesize(mount->bytes_used, m_fixed ? 2 : 0, m_fixed, m_bar.locale));
       builder->node(m_labelmounted);
     } else if (tag == TAG_LABEL_UNMOUNTED) {
       m_labelunmounted->reset_tokens();
diff --git a/src/modules/memory.cpp b/src/modules/memory.cpp
index eeb782b1..82fe5b36 100644
--- a/src/modules/memory.cpp
+++ b/src/modules/memory.cpp
@@ -2,10 +2,10 @@
 #include <iomanip>
 #include <istream>
 
-#include "modules/memory.hpp"
-
 #include "drawtypes/label.hpp"
 #include "drawtypes/progressbar.hpp"
+#include "modules/memory.hpp"
+#include "utils/math.hpp"
 
 #include "modules/meta/base.inl"
 
@@ -20,10 +20,10 @@ namespace modules {
     m_formatter->add(DEFAULT_FORMAT, TAG_LABEL, {TAG_LABEL, TAG_BAR_USED, TAG_BAR_FREE});
 
     if (m_formatter->has(TAG_BAR_USED)) {
-      m_bars[memtype::USED] = load_progressbar(m_bar, m_conf, name(), TAG_BAR_USED);
+      m_bar_memused = load_progressbar(m_bar, m_conf, name(), TAG_BAR_USED);
     }
     if (m_formatter->has(TAG_BAR_FREE)) {
-      m_bars[memtype::FREE] = load_progressbar(m_bar, m_conf, name(), TAG_BAR_FREE);
+      m_bar_memfree = load_progressbar(m_bar, m_conf, name(), TAG_BAR_FREE);
     }
     if (m_formatter->has(TAG_LABEL)) {
       m_label = load_optional_label(m_conf, name(), TAG_LABEL, "%percentage_used%");
@@ -31,66 +31,45 @@ namespace modules {
   }
 
   bool memory_module::update() {
-    float kb_total;
-    float kb_avail;
-    // long kb_free;
+    unsigned long long kb_total{0ULL};
+    unsigned long long kb_avail{0ULL};
+    unsigned long long kb_free{0ULL};
 
     try {
       std::ifstream in(PATH_MEMORY_INFO);
       std::stringstream buffer;
-      string str, rdbuf;
-      int i = 0;
-
-      in.exceptions(in.failbit);
+      string str;
 
       buffer.imbue(std::locale::classic());
 
-      while (std::getline(in, str) && i++ < 3) {
+      for (int i = 3; i > 0 && std::getline(in, str); i--) {
         size_t off = str.find_first_of("1234567890", str.find(':'));
         if (off != string::npos && str.size() > off) {
-          buffer << std::strtol(&str[off], nullptr, 10) << std::endl;
+          buffer << std::strtoull(&str[off], nullptr, 10) << std::endl;
         }
       }
 
-      buffer >> rdbuf;
-      kb_total = std::atol(rdbuf.c_str());
-      buffer >> rdbuf;  // kb_free = std::atol(rdbuf.c_str());
-      buffer >> rdbuf;
-      kb_avail = std::atol(rdbuf.c_str());
-    } catch (const std::ios_base::failure& e) {
-      kb_total = 0;
-      // kb_free = 0;
-      kb_avail = 0;
-      m_log.err("Failed to read memory values (what: %s)", e.what());
+      buffer >> kb_total;
+      buffer >> kb_free;
+      buffer >> kb_avail;
+    } catch (const std::exception& err) {
+      m_log.err("Failed to read memory values (what: %s)", err.what());
     }
 
-    if (kb_total > 0) {
-      m_perc[memtype::FREE] = (kb_avail / kb_total) * 100.0f + 0.5f;
-    } else {
-      m_perc[memtype::FREE] = 0;
-    }
-
-    m_perc[memtype::USED] = 100 - m_perc[memtype::FREE];
+    m_perc_memfree = math_util::percentage(kb_avail, kb_total);
+    m_perc_memused = 100 - m_perc_memfree;
 
     // replace tokens
     if (m_label) {
       m_label->reset_tokens();
-
-      auto replace_unit = [](label_t& label, string token, float value, string unit) {
-        auto formatted =
-            string_util::from_stream(stringstream() << std::setprecision(2) << std::fixed << value << " " << unit);
-        label->replace_token(token, formatted);
-      };
-
-      replace_unit(m_label, "%gb_used%", (kb_total - kb_avail) / 1024 / 1024, "GB");
-      replace_unit(m_label, "%gb_free%", kb_avail / 1024 / 1024, "GB");
-      replace_unit(m_label, "%gb_total%", kb_total / 1024 / 1024, "GB");
-      replace_unit(m_label, "%mb_used%", (kb_total - kb_avail) / 1024, "MB");
-      replace_unit(m_label, "%mb_free%", kb_avail / 1024, "MB");
-      replace_unit(m_label, "%mb_total%", kb_total / 1024, "MB");
-
-      m_label->replace_token("%percentage_used%", to_string(m_perc[memtype::USED]) + "%");
-      m_label->replace_token("%percentage_free%", to_string(m_perc[memtype::FREE]) + "%");
+      m_label->replace_token("%gb_used%", string_util::filesize_gb(kb_total - kb_avail, 2, m_bar.locale));
+      m_label->replace_token("%gb_free%", string_util::filesize_gb(kb_avail, 2, m_bar.locale));
+      m_label->replace_token("%gb_total%", string_util::filesize_gb(kb_total, 2, m_bar.locale));
+      m_label->replace_token("%mb_used%", string_util::filesize_mb(kb_total - kb_avail, 2, m_bar.locale));
+      m_label->replace_token("%mb_free%", string_util::filesize_mb(kb_avail, 2, m_bar.locale));
+      m_label->replace_token("%mb_total%", string_util::filesize_mb(kb_total, 2, m_bar.locale));
+      m_label->replace_token("%percentage_used%", to_string(m_perc_memused) + "%");
+      m_label->replace_token("%percentage_free%", to_string(m_perc_memfree) + "%");
     }
 
     return true;
@@ -98,9 +77,9 @@ namespace modules {
 
   bool memory_module::build(builder* builder, const string& tag) const {
     if (tag == TAG_BAR_USED) {
-      builder->node(m_bars.at(memtype::USED)->output(m_perc.at(memtype::USED)));
+      builder->node(m_bar_memused->output(m_perc_memused));
     } else if (tag == TAG_BAR_FREE) {
-      builder->node(m_bars.at(memtype::FREE)->output(m_perc.at(memtype::FREE)));
+      builder->node(m_bar_memfree->output(m_perc_memfree));
     } else if (tag == TAG_LABEL) {
       builder->node(m_label);
     } else {
diff --git a/src/utils/string.cpp b/src/utils/string.cpp
index d44f3dac..29827e24 100644
--- a/src/utils/string.cpp
+++ b/src/utils/string.cpp
@@ -1,4 +1,5 @@
 #include <cstring>
+#include <iomanip>
 #include <sstream>
 #include <utility>
 
@@ -187,43 +188,41 @@ namespace string_util {
   }
 
   /**
-   * Create a float value string
+   * Create a floating point string
    */
-  string floatval(float value, int decimals, bool fixed, const string& locale) {
+  string floating_point(double value, size_t precision, bool fixed, const string& locale) {
     stringstream ss;
-    ss.precision(decimals);
-    if (!locale.empty()) {
-      ss.imbue(std::locale(locale.c_str()));
-    }
-    if (fixed) {
-      ss << std::fixed;
-    }
-    ss << value;
-    return ss.str();
+    ss.imbue(!locale.empty() ? std::locale(locale.c_str()) : std::locale::classic());
+    ss << std::fixed << std::setprecision(precision) << value;
+    return fixed ? ss.str() : replace(ss.str(), ".00", "");
   }
 
   /**
-   * Format a filesize string
+   * Create a MB filesize string
    */
-  string filesize(unsigned long long bytes, int decimals, bool fixed, const string& locale) {
+  string filesize_mb(unsigned long long kbytes, size_t precision, const string& locale) {
+    return floating_point(kbytes / 1024.0, precision, true, locale) + " MB";
+  }
+
+  /**
+   * Create a GB filesize string
+   */
+  string filesize_gb(unsigned long long kbytes, size_t precision, const string& locale) {
+    return floating_point(kbytes / 1024.0 / 1024.0, precision, true, locale) + " GB";
+  }
+
+  /**
+   * Create a filesize string by converting given bytes to highest unit possible
+   */
+  string filesize(unsigned long long kbytes, size_t precision, bool fixed, const string& locale) {
     vector<string> suffixes{"TB", "GB", "MB"};
     string suffix{"KB"};
-
-    while ((bytes /= 1024) >= 1024) {
+    double value = kbytes;
+    while (!suffixes.empty() && (value /= 1024.0) >= 1024.0) {
       suffix = suffixes.back();
       suffixes.pop_back();
     }
-
-    stringstream ss;
-    ss.precision(decimals);
-    if (!locale.empty()) {
-      ss.imbue(std::locale(locale.c_str()));
-    }
-    if (fixed) {
-      ss << std::fixed;
-    }
-    ss << bytes << " " << suffix;
-    return ss.str();
+    return floating_point(value, precision, fixed, locale) + " GB";
   }
 
   /**
diff --git a/tests/unit_tests/utils/string.cpp b/tests/unit_tests/utils/string.cpp
index ad0ef448..aebd4ce7 100644
--- a/tests/unit_tests/utils/string.cpp
+++ b/tests/unit_tests/utils/string.cpp
@@ -89,4 +89,21 @@ int main() {
     expect(hashA1 != hashB2);
     expect(hashB1 != hashB2);
   };
+
+  "floating_point"_test = [] {
+    expect(string_util::floating_point(1.2599, 2) == "1.26");
+    expect(string_util::floating_point(1.7, 0) == "2");
+    expect(string_util::floating_point(1.777, 10) == "1.7770000000");
+  };
+
+  "filesize"_test = [] {
+    expect(string_util::filesize_mb(3 * 1024, 3) == "3.000 MB");
+    expect(string_util::filesize_mb(3 * 1024 + 200, 3) == "3.195 MB");
+    expect(string_util::filesize_mb(3 * 1024 + 400) == "3 MB");
+    expect(string_util::filesize_mb(3 * 1024 + 800) == "4 MB");
+    expect(string_util::filesize_gb(3 * 1024 * 1024 + 200 * 1024, 3) == "3.195 GB");
+    expect(string_util::filesize_gb(3 * 1024 * 1024 + 400 * 1024) == "3 GB");
+    expect(string_util::filesize_gb(3 * 1024 * 1024 + 800 * 1024) == "4 GB");
+    expect(string_util::filesize(3 * 1024 * 1024) == "3 GB");
+  };
 }