commit 0ae67ba02d2f2cdba713889f6c2688825e4c8a94
parent 9f84283940691305d5942ef6efc6ba7d7192abd1
Author: cfillion <cfillion@users.noreply.github.com>
Date: Wed, 24 Aug 2016 01:42:04 -0400
refactoring – drop duplicate sources
Diffstat:
12 files changed, 36 insertions(+), 98 deletions(-)
diff --git a/src/about.cpp b/src/about.cpp
@@ -494,23 +494,12 @@ void AboutPackageDelegate::updateList(const int index)
stream << *ver;
SetWindowText(m_dialog->report(), make_autostring(stream.str()).c_str());
- vector<const Source *> uniqueSources;
+ m_sources = &ver->sources();
- const auto &sources = ver->sources();
- for(auto it = sources.begin(); it != sources.end();) {
- const Path &path = it->first;
- const Source *src = it->second;
-
- m_dialog->list()->addRow({make_autostring(path.join()),
+ for(const Source *src : ver->sources()) {
+ m_dialog->list()->addRow({make_autostring(src->targetPath().join()),
make_autostring(src->isMain() ? "Yes" : "No")});
-
- uniqueSources.push_back(src);
-
- // skip duplicate files
- do { it++; } while(it != sources.end() && path == it->first);
}
-
- swap(m_sources, uniqueSources);
}
bool AboutPackageDelegate::fillContextMenu(Menu &menu, const int index) const
@@ -542,5 +531,5 @@ void AboutPackageDelegate::copySourceUrl()
if(index < 0)
return;
- m_dialog->setClipboard(m_sources[index]->url());
+ m_dialog->setClipboard(m_sources->at(index)->url());
}
diff --git a/src/about.hpp b/src/about.hpp
@@ -128,7 +128,7 @@ private:
const Package *m_package;
ReaPack *m_reapack;
IndexPtr m_index; // keeps the package loaded in memory
- std::vector<const Source *> m_sources;
+ const std::vector<const Source *> *m_sources;
About *m_dialog;
};
diff --git a/src/browser.hpp b/src/browser.hpp
@@ -36,6 +36,7 @@ class Index;
class ListView;
class Menu;
class ReaPack;
+class Remote;
class Version;
typedef std::shared_ptr<const Index> IndexPtr;
diff --git a/src/package.hpp b/src/package.hpp
@@ -24,9 +24,6 @@
class Category;
-class Package;
-typedef std::vector<const Package *> PackageList;
-
class Package {
public:
enum Type {
@@ -75,4 +72,6 @@ private:
VersionSet m_versions;
};
+typedef std::vector<const Package *> PackageList;
+
#endif
diff --git a/src/registry.cpp b/src/registry.cpp
@@ -167,10 +167,8 @@ auto Registry::push(const Version *ver, vector<Path> *conflicts) -> Entry
}
// register files
- const auto &sources = ver->sources();
- for(auto it = sources.begin(); it != sources.end();) {
- const Path &path = it->first;
- const Source *src = it->second;
+ for(const Source *src : ver->sources()) {
+ const Path &path = src->targetPath();
m_insertFile->bind(1, entryId);
m_insertFile->bind(2, path.join('/'));
@@ -190,9 +188,6 @@ auto Registry::push(const Version *ver, vector<Path> *conflicts) -> Entry
throw;
}
}
-
- // skip duplicate files
- do { it++; } while(it != sources.end() && path == it->first);
}
if(hasConflicts) {
diff --git a/src/registry.hpp b/src/registry.hpp
@@ -21,14 +21,10 @@
#include <set>
#include <string>
-#include "path.hpp"
#include "database.hpp"
#include "package.hpp"
-
-class Package;
-class Path;
-class Remote;
-class Version;
+#include "path.hpp"
+#include "version.hpp"
class Registry {
public:
diff --git a/src/task.cpp b/src/task.cpp
@@ -60,20 +60,12 @@ bool InstallTask::start()
return false;
}
- const auto &sources = m_version->sources();
-
- for(auto it = sources.begin(); it != sources.end();) {
- const Path &path = it->first;
- const Source *src = it->second;
-
+ for(const Source *src : m_version->sources()) {
const NetworkOpts &opts = tx()->config()->network;
Download *dl = new Download(src->fullName(), src->url(), opts);
dl->onFinish(bind(&InstallTask::saveSource, this, dl, src));
tx()->downloadQueue()->push(dl);
-
- // skip duplicate files
- do { it++; } while(it != sources.end() && path == it->first);
}
return true;
diff --git a/src/version.cpp b/src/version.cpp
@@ -48,8 +48,8 @@ Version::Version(const Version &o, const Package *pkg)
Version::~Version()
{
- for(const auto &pair : m_sources)
- delete pair.second;
+ for(const Source *source : m_sources)
+ delete source;
}
void Version::parse(const string &str)
@@ -124,23 +124,14 @@ bool Version::addSource(const Source *source)
return false;
const Path path = source->targetPath();
- m_files.insert(path);
- m_sources.insert({path, source});
-
- if(source->isMain())
- m_mainSources.push_back(source);
-
- return true;
-}
-const Source *Version::source(const size_t index) const
-{
- auto it = m_sources.begin();
+ if(m_files.count(path))
+ return false;
- for(size_t i = 0; i < index; i++)
- it++;
+ m_files.insert(path);
+ m_sources.push_back(source);
- return it->second;
+ return true;
}
auto Version::segment(const size_t index) const -> Segment
diff --git a/src/version.hpp b/src/version.hpp
@@ -34,7 +34,6 @@ class Path;
class Version {
public:
typedef std::vector<const Source *> SourceList;
- typedef std::multimap<Path, const Source *> SourceMap;
Version();
Version(const std::string &, const Package * = nullptr);
@@ -62,9 +61,8 @@ public:
const std::string &changelog() const { return m_changelog; }
bool addSource(const Source *source);
- const SourceMap &sources() const { return m_sources; }
- const Source *source(size_t) const;
- const SourceList &mainSources() const { return m_mainSources; }
+ const SourceList &sources() const { return m_sources; }
+ const Source *source(size_t i) const { return m_sources[i]; }
const std::set<Path> &files() const { return m_files; }
@@ -91,13 +89,12 @@ private:
Time m_time;
const Package *m_package;
- SourceList m_mainSources;
- SourceMap m_sources;
+ SourceList m_sources;
std::set<Path> m_files;
};
-class VersionPtrCompare {
+class CompareVersionPtr {
public:
bool operator()(const Version *l, const Version *r) const
{
@@ -105,6 +102,6 @@ public:
}
};
-typedef std::set<const Version *, VersionPtrCompare> VersionSet;
+typedef std::set<const Version *, CompareVersionPtr> VersionSet;
#endif
diff --git a/test/index_v1.cpp b/test/index_v1.cpp
@@ -155,36 +155,33 @@ TEST_CASE("full index", M) {
UseRootPath root(RIPATH);
IndexPtr ri = Index::load("valid_index");
- REQUIRE(ri->categories().size() == 1);
+ REQUIRE(ri->categories().size() == 1);
const Category *cat = ri->category(0);
REQUIRE(cat->name() == "Category Name");
- REQUIRE(cat->packages().size() == 1);
+ REQUIRE(cat->packages().size() == 1);
const Package *pack = cat->package(0);
REQUIRE(pack->type() == Package::ScriptType);
REQUIRE(pack->name() == "Hello World.lua");
- REQUIRE(pack->versions().size() == 1);
+ REQUIRE(pack->versions().size() == 1);
const Version *ver = pack->version(0);
REQUIRE(ver->name() == "1.0");
- REQUIRE(ver->sources().size() == 2);
REQUIRE(ver->changelog() == "Fixed a division by zero error.");
- const Source *source1 = ver->source(1);
+ REQUIRE(ver->sources().size() == 2);
+ const Source *source1 = ver->source(0);
REQUIRE(source1->platform() == Platform::GenericPlatform);
REQUIRE(source1->file() == "test.lua");
REQUIRE(source1->isMain());
REQUIRE(source1->url() == "https://google.com/");
- const Source *source2 = ver->source(0);
+ const Source *source2 = ver->source(1);
REQUIRE(source2->platform() == Platform::GenericPlatform);
REQUIRE(source2->file() == "background.png");
REQUIRE_FALSE(source2->isMain());
REQUIRE(source2->url() == "http://cfillion.tk/");
-
- REQUIRE(ver->mainSources().size() == 1);
- REQUIRE(ver->mainSources()[0] == source1);
}
TEST_CASE("read index metadata", M) {
diff --git a/test/source.cpp b/test/source.cpp
@@ -36,6 +36,8 @@ TEST_CASE("source type from package", M) {
Version ver("1.0", &pack);
Source src({}, "url", &ver);
+ REQUIRE(src.version() == &ver);
+
REQUIRE(src.type() == Package::EffectType);
src.setTypeOverride(Package::ScriptType);
REQUIRE(src.type() == Package::ScriptType);
diff --git a/test/version.cpp b/test/version.cpp
@@ -25,7 +25,6 @@ TEST_CASE("construct null version", M) {
REQUIRE(ver.isStable());
REQUIRE_FALSE(ver.time().isValid());
REQUIRE(ver.package() == nullptr);
- REQUIRE(ver.mainSources().empty());
}
TEST_CASE("compare null versions", M) {
@@ -230,9 +229,6 @@ TEST_CASE("add source", M) {
REQUIRE(ver.addSource(src));
CHECK(ver.sources().size() == 1);
- CHECK(ver.mainSources().empty());
-
- REQUIRE(src->version() == &ver);
REQUIRE(ver.source(0) == src);
}
@@ -252,30 +248,14 @@ TEST_CASE("add owned source", M) {
}
}
-TEST_CASE("add main sources", M) {
- MAKE_VERSION
-
- Source *src1 = new Source({}, "b", &ver);
- src1->setMain(true);
- ver.addSource(src1);
-
- Source *src2 = new Source({}, "b", &ver);
- src2->setMain(true);
- ver.addSource(src2);
-
- REQUIRE(ver.mainSources().size() == 2);
- REQUIRE(ver.mainSources()[0] == src1);
- REQUIRE(ver.mainSources()[1] == src2);
-}
-
TEST_CASE("duplicate sources", M) {
MAKE_VERSION
Source *src = new Source({}, "b", &ver);
- ver.addSource(src);
- ver.addSource(new Source({}, "b", &ver));
+ CHECK(ver.addSource(src) == true);
+ REQUIRE(ver.addSource(src) == false);
- REQUIRE(ver.sources().size() == 2);
+ REQUIRE(ver.sources().size() == 1);
REQUIRE(ver.source(0) == src);
}
@@ -337,7 +317,6 @@ TEST_CASE("copy version constructor", M) {
REQUIRE(copy1.changelog() == original.changelog());
REQUIRE(copy1.time() == original.time());
REQUIRE(copy1.package() == nullptr);
- REQUIRE(copy1.mainSources().empty());
REQUIRE(copy1.sources().empty());
const Version copy2(original, &pkg);
