From 18871b4b15139c26f97a7aa0b0d07b76ec95bf5c Mon Sep 17 00:00:00 2001
From: Veloman Yunkan <veloman.yunkan@gmail.com>
Date: Thu, 10 Jun 2021 12:27:51 +0400
Subject: [PATCH] Helper function Library::getBookPropValueSet()

Introduced a helper function `Library::getBookPropValueSet()` and
deduplicated Library::getBooks{Languages,Creators,Publishers}() methods.
---
 include/library.h |  4 ++++
 src/library.cpp   | 56 ++++++++++++-----------------------------------
 test/library.cpp  | 22 ++++++++++++++++---
 3 files changed, 37 insertions(+), 45 deletions(-)

diff --git a/include/library.h b/include/library.h
index c5e026e2f..f1ba7849c 100644
--- a/include/library.h
+++ b/include/library.h
@@ -341,7 +341,11 @@ class Library
   friend class OPDSDumper;
   friend class libXMLDumper;
 
+private: // types
+  typedef const std::string& (Book::*BookStrPropMemFn)() const;
+
 private: // functions
+  std::vector<std::string> getBookPropValueSet(BookStrPropMemFn p) const;
   BookIdCollection filterViaBookDB(const Filter& filter) const;
   void updateBookDB(const Book& book);
 };
diff --git a/src/library.cpp b/src/library.cpp
index 3a0f0cd53..6633dce5f 100644
--- a/src/library.cpp
+++ b/src/library.cpp
@@ -208,23 +208,23 @@ bool Library::writeBookmarksToFile(const std::string& path) const
   return writeTextFile(path, dumper.dumpLibXMLBookmark());
 }
 
-std::vector<std::string> Library::getBooksLanguages() const
+std::vector<std::string> Library::getBookPropValueSet(BookStrPropMemFn p) const
 {
-  std::vector<std::string> booksLanguages;
-  std::map<std::string, bool> booksLanguagesMap;
+  std::set<std::string> propValues;
 
-  for (auto& pair: m_books) {
-    auto& book = pair.second;
-    auto& language = book.getLanguage();
-    if (booksLanguagesMap.find(language) == booksLanguagesMap.end()) {
-      if (book.getOrigId().empty()) {
-        booksLanguagesMap[language] = true;
-        booksLanguages.push_back(language);
-      }
+  for (const auto& pair: m_books) {
+    const auto& book = pair.second;
+    if (book.getOrigId().empty()) {
+      propValues.insert((book.*p)());
     }
   }
 
-  return booksLanguages;
+  return std::vector<std::string>(propValues.begin(), propValues.end());
+}
+
+std::vector<std::string> Library::getBooksLanguages() const
+{
+  return getBookPropValueSet(&Book::getLanguage);
 }
 
 std::vector<std::string> Library::getBooksCategories() const
@@ -244,40 +244,12 @@ std::vector<std::string> Library::getBooksCategories() const
 
 std::vector<std::string> Library::getBooksCreators() const
 {
-  std::vector<std::string> booksCreators;
-  std::map<std::string, bool> booksCreatorsMap;
-
-  for (auto& pair: m_books) {
-    auto& book = pair.second;
-    auto& creator = book.getCreator();
-    if (booksCreatorsMap.find(creator) == booksCreatorsMap.end()) {
-      if (book.getOrigId().empty()) {
-        booksCreatorsMap[creator] = true;
-        booksCreators.push_back(creator);
-      }
-    }
-  }
-
-  return booksCreators;
+  return getBookPropValueSet(&Book::getCreator);
 }
 
 std::vector<std::string> Library::getBooksPublishers() const
 {
-  std::vector<std::string> booksPublishers;
-  std::map<std::string, bool> booksPublishersMap;
-
-  for (auto& pair:m_books) {
-    auto& book = pair.second;
-    auto& publisher = book.getPublisher();
-    if (booksPublishersMap.find(publisher) == booksPublishersMap.end()) {
-      if (book.getOrigId().empty()) {
-        booksPublishersMap[publisher] = true;
-        booksPublishers.push_back(publisher);
-      }
-    }
-  }
-
-  return booksPublishers;
+  return getBookPropValueSet(&Book::getPublisher);
 }
 
 const std::vector<kiwix::Bookmark> Library::getBookmarks(bool onlyValidBookmarks) const
diff --git a/test/library.cpp b/test/library.cpp
index 543f01594..77780ba7b 100644
--- a/test/library.cpp
+++ b/test/library.cpp
@@ -275,9 +275,25 @@ TEST_F(LibraryTest, getBookMarksTest)
 TEST_F(LibraryTest, sanityCheck)
 {
   EXPECT_EQ(lib.getBookCount(true, true), 12U);
-  EXPECT_EQ(lib.getBooksLanguages().size(), 3U);
-  EXPECT_EQ(lib.getBooksCreators().size(), 9U);
-  EXPECT_EQ(lib.getBooksPublishers().size(), 3U);
+  EXPECT_EQ(lib.getBooksLanguages(),
+            std::vector<std::string>({"deu", "eng", "fra"})
+  );
+  EXPECT_EQ(lib.getBooksCreators(), std::vector<std::string>({
+      "Islam Stack Exchange",
+      "Movies & TV Stack Exchange",
+      "Mythology & Folklore Stack Exchange",
+      "TED",
+      "Tania Louis",
+      "Wiki",
+      "Wikibooks",
+      "Wikipedia",
+      "Wikiquote"
+  }));
+  EXPECT_EQ(lib.getBooksPublishers(), std::vector<std::string>({
+      "",
+      "Kiwix",
+      "Kiwix & Some Enthusiasts"
+  }));
 }
 
 TEST_F(LibraryTest, categoryHandling)