- make Files class responsible for sorting entries

openSUSE · Apr 22, 2022 · 1eadd2f · 1eadd2f
1 parent 9a623ab
commit 1eadd2f
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 30 deletions.
diff --git a/client/commands.cc b/client/commands.cc
@@ -466,13 +466,6 @@ command_delete_comparison(DBus::Connection& conn, const string& config_name, uns
 }
 
 
-int
-operator<(const XFile& lhs, const XFile& rhs)
-{
-    return File::cmp_lt(lhs.name, rhs.name);
-}
-
-
 vector<XFile>
 command_get_xfiles(DBus::Connection& conn, const string& config_name, unsigned int number1,
 		   unsigned int number2)
@@ -489,8 +482,6 @@ command_get_xfiles(DBus::Connection& conn, const string& config_name, unsigned i
     DBus::Hihi hihi(reply);
     hihi >> files;
 
-    sort(files.begin(), files.end());
-
     return files;
 }
 
@@ -553,7 +544,6 @@ command_get_xfiles_by_pipe(DBus::Connection& conn, const string& config_name, un
     if (fclose(fin) != 0)
 	SN_THROW(IOErrorException("reading pipe failed, fclose failed: " + stringerror(errno)));
 
-    sort(files.begin(), files.end());
 
     return files;
 }

diff --git a/snapper/File.cc b/snapper/File.cc
@@ -74,6 +74,24 @@ namespace snapper
     }
 
 
+    Files::Files(const FilePaths* file_paths)
+	: file_paths(file_paths)
+    {
+    }
+
+
+    Files::Files(const FilePaths* file_paths, const vector<File>& entries)
+	: file_paths(file_paths), entries(entries)
+    {
+	sort();
+    }
+
+
+    Files::~Files()
+    {
+    }
+
+
     void
     Files::filter(const vector<string>& ignore_patterns)
     {
@@ -89,7 +107,7 @@ namespace snapper
 
 
     bool
-    File::cmp_lt(const string& lhs, const string& rhs)
+    cmp_lt(const string& lhs, const string& rhs)
     {
 	const std::collate<char>& c = std::use_facet<std::collate<char>>(std::locale());
 
@@ -108,7 +126,7 @@ namespace snapper
     bool
     operator<(const File& lhs, const File& rhs)
     {
-	return File::cmp_lt(lhs.getName(), rhs.getName());
+	return cmp_lt(lhs.getName(), rhs.getName());
     }
 
 
@@ -122,7 +140,7 @@ namespace snapper
     bool
     operator<(const File& file, const string& name)
     {
-	return File::cmp_lt(file.getName(), name);
+	return cmp_lt(file.getName(), name);
     }
 
 

diff --git a/snapper/File.h b/snapper/File.h
@@ -136,9 +136,6 @@ namespace snapper
 
 	XAUndoStatistic getXAUndoStatistic() const;
 
-	// C++ locale aware less-than comparison
-	static bool cmp_lt(const string& lhs, const string& rhs);
-
     private:
 
 	bool createParentDirectories(const string& path) const;
@@ -172,17 +169,20 @@ namespace snapper
     };
 
 
+    /**
+     * Container class for files.
+     *
+     * The Files class keeps the files sorted, which is required for the find functions.
+     */
     class Files
     {
     public:
 
 	friend class Comparison;
 
-	Files(const FilePaths* file_paths)
-	    : file_paths(file_paths) {}
-
-	Files(const FilePaths* file_paths, const vector<File>& entries)
-	    : file_paths(file_paths), entries(entries) {}
+	Files(const FilePaths* file_paths);
+	Files(const FilePaths* file_paths, const vector<File>& entries);
+	~Files();
 
 	typedef vector<File>::iterator iterator;
 	typedef vector<File>::const_iterator const_iterator;
@@ -213,17 +213,18 @@ namespace snapper
 
 	XAUndoStatistic getXAUndoStatistic() const;
 
-    protected:
+    private:
 
+	/**
+	 * After using push_back, sort must be called before using any find function.
+	 */
 	void push_back(const File& file) { entries.push_back(file); }
 
-	void filter(const vector<string>& ignore_patterns);
-
 	void sort();
 
-	const FilePaths* file_paths;
+	void filter(const vector<string>& ignore_patterns);
 
-    private:
+	const FilePaths* file_paths;
 
 	vector<File> entries;
 

diff --git a/testsuite/cmp-lt.cc b/testsuite/cmp-lt.cc
@@ -26,13 +26,18 @@ namespace std
     }
 }
 
+namespace snapper
+{
+    bool cmp_lt(const string& lhs, const string& rhs);
+}
+
 
 BOOST_AUTO_TEST_CASE(test1)
 {
     std::locale::global(std::locale("C"));
 
     vector<string> v = { "A", "B", "b", "a" };
-    sort(v.begin(), v.end(), File::cmp_lt);
+    sort(v.begin(), v.end(), cmp_lt);
 
     BOOST_CHECK_EQUAL(v, vector<string>({ "A", "B", "a", "b" }));
 }
@@ -43,7 +48,7 @@ BOOST_AUTO_TEST_CASE(test2)
     std::locale::global(std::locale("en_US.UTF-8"));
 
     vector<string> v = { "A", "B", "b", "a" };
-    sort(v.begin(), v.end(), File::cmp_lt);
+    sort(v.begin(), v.end(), cmp_lt);
 
     BOOST_CHECK_EQUAL(v, vector<string>({ "a", "A", "b", "B" }));
 }
@@ -54,7 +59,7 @@ BOOST_AUTO_TEST_CASE(test3)
     std::locale::global(std::locale("de_DE.UTF-8"));
 
     vector<string> v = { "a", "b", "ä" };
-    sort(v.begin(), v.end(), File::cmp_lt);
+    sort(v.begin(), v.end(), cmp_lt);
 
     BOOST_CHECK_EQUAL(v, vector<string>({ "a", "ä", "b" }));
 }
@@ -65,7 +70,7 @@ BOOST_AUTO_TEST_CASE(test4)
     std::locale::global(std::locale("en_US.UTF-8"));
 
     vector<string> v = { "a", "\344" }; // invalid UTF-8
-    sort(v.begin(), v.end(), File::cmp_lt);
+    sort(v.begin(), v.end(), cmp_lt);
 
     BOOST_CHECK_EQUAL(v, vector<string>({ "\344", "a" }));
 }