From 0a366926056a1047a6c7040b04c62c2e8ce2a0ea Mon Sep 17 00:00:00 2001 From: vng Date: Wed, 7 Aug 2013 14:33:51 +0300 Subject: [PATCH] Factor out StorageBuilder from Storage code. --- android/jni/Android.mk | 1 + android/jni/and_storage.cpp | 4 +- iOS/offlineguides.xcodeproj/project.pbxproj | 4 + iOS/offlineguides/ArticleVC.mm | 9 +- storage/article_info.cpp | 12 +-- storage/article_info.hpp | 13 +-- storage/storage.cpp | 64 -------------- storage/storage.hpp | 33 ++++---- storage/storage.pro | 2 + storage/storage_builder.cpp | 94 +++++++++++++++++++++ storage/storage_builder.hpp | 49 +++++++++++ storage/tests/storage_test.cpp | 53 +++++------- 12 files changed, 208 insertions(+), 130 deletions(-) create mode 100644 storage/storage_builder.cpp create mode 100644 storage/storage_builder.hpp diff --git a/android/jni/Android.mk b/android/jni/Android.mk index 60706a2..9b1d20b 100644 --- a/android/jni/Android.mk +++ b/android/jni/Android.mk @@ -29,5 +29,6 @@ LOCAL_SRC_FILES += \ ../../storage/article_info.cpp \ ../../storage/distance.cpp \ ../../storage/storage.cpp \ + ../../storage/storage_builder.cpp \ include $(BUILD_SHARED_LIBRARY) diff --git a/android/jni/and_storage.cpp b/android/jni/and_storage.cpp index 1ed4506..4d69fde 100644 --- a/android/jni/and_storage.cpp +++ b/android/jni/and_storage.cpp @@ -1,7 +1,8 @@ #include "and_storage.hpp" #include "jni_util.hpp" -#include "../../storage/storage.hpp" +/// @todo Replace on storage.hpp +#include "../../storage/storage_builder.hpp" class AndStorage @@ -33,6 +34,7 @@ public: } private: + /// @todo Replace on Storage StorageMock m_storage; vector m_result; }; diff --git a/iOS/offlineguides.xcodeproj/project.pbxproj b/iOS/offlineguides.xcodeproj/project.pbxproj index aad7be7..5a45782 100644 --- a/iOS/offlineguides.xcodeproj/project.pbxproj +++ b/iOS/offlineguides.xcodeproj/project.pbxproj @@ -9,6 +9,7 @@ /* Begin PBXBuildFile section */ 7722E14817B01B0700CFB817 /* article_info.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 7722E14717B01B0700CFB817 /* article_info.cpp */; }; 7722E14A17B15C3600CFB817 /* distance.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 7722E14917B15C3600CFB817 /* distance.cpp */; }; + 7722E14C17B2638B00CFB817 /* storage_builder.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 7722E14B17B2638B00CFB817 /* storage_builder.cpp */; }; A3E221F317AFB4C20018AB9E /* storage.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A3E221F217AFB4C20018AB9E /* storage.cpp */; }; A3E2220C17AFD5840018AB9E /* data in Resources */ = {isa = PBXBuildFile; fileRef = A3E2220B17AFD5840018AB9E /* data */; }; A3E2220F17AFD9E10018AB9E /* assert.cpp in Sources */ = {isa = PBXBuildFile; fileRef = A3E2220E17AFD9E10018AB9E /* assert.cpp */; }; @@ -32,6 +33,7 @@ /* Begin PBXFileReference section */ 7722E14717B01B0700CFB817 /* article_info.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = article_info.cpp; path = ../../storage/article_info.cpp; sourceTree = ""; }; 7722E14917B15C3600CFB817 /* distance.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = distance.cpp; path = ../../storage/distance.cpp; sourceTree = ""; }; + 7722E14B17B2638B00CFB817 /* storage_builder.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = storage_builder.cpp; path = ../../storage/storage_builder.cpp; sourceTree = ""; }; A3E221F217AFB4C20018AB9E /* storage.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = storage.cpp; path = ../../storage/storage.cpp; sourceTree = ""; }; A3E2220B17AFD5840018AB9E /* data */ = {isa = PBXFileReference; lastKnownFileType = folder; name = data; path = ../../data; sourceTree = ""; }; A3E2220E17AFD9E10018AB9E /* assert.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = assert.cpp; path = ../../env/assert.cpp; sourceTree = ""; }; @@ -75,6 +77,7 @@ A3E221ED17AFB49B0018AB9E /* storage */ = { isa = PBXGroup; children = ( + 7722E14B17B2638B00CFB817 /* storage_builder.cpp */, 7722E14917B15C3600CFB817 /* distance.cpp */, 7722E14717B01B0700CFB817 /* article_info.cpp */, A3E221F217AFB4C20018AB9E /* storage.cpp */, @@ -238,6 +241,7 @@ ED81956E17B10578004F3803 /* strings.cpp in Sources */, ED81957117B10598004F3803 /* utf8proc.c in Sources */, 7722E14A17B15C3600CFB817 /* distance.cpp in Sources */, + 7722E14C17B2638B00CFB817 /* storage_builder.cpp in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/iOS/offlineguides/ArticleVC.mm b/iOS/offlineguides/ArticleVC.mm index 6fa8a08..d8e9b09 100644 --- a/iOS/offlineguides/ArticleVC.mm +++ b/iOS/offlineguides/ArticleVC.mm @@ -1,18 +1,17 @@ #import "ArticleVC.h" #import "GuideVC.h" -#import "../../storage/storage.hpp" -#import "../../storage/article_info.hpp" +/// @todo Replace on storage.hpp +#import "../../storage/storage_builder.hpp" #import "../../env/assert.hpp" -#import "../../std/vector.hpp" - #define THUMBNAILSFOLDER @"/data/thumbnails/" @interface ArticleVC () { + /// @todo Replace on Storage StorageMock m_storage; vector m_infos; } @@ -70,7 +69,7 @@ string imageType = info->m_thumbnailUrl.substr(pos+1); NSString * imagePath = [[NSBundle mainBundle] pathForResource:[NSString stringWithUTF8String:imageName.c_str()] ofType:[NSString stringWithUTF8String:imageType.c_str()] inDirectory:THUMBNAILSFOLDER]; - cell.detailTextLabel.text = [NSString stringWithUTF8String:info->m_parentUrl.c_str()]; + cell.detailTextLabel.text = [NSString stringWithUTF8String:m_storage.GetParentName(*info).c_str()]; UIImage * image = [UIImage imageWithContentsOfFile:imagePath]; cell.imageView.image = image; diff --git a/storage/article_info.cpp b/storage/article_info.cpp index bae3ee1..e8ac025 100644 --- a/storage/article_info.cpp +++ b/storage/article_info.cpp @@ -38,7 +38,7 @@ void ArticleInfo::Write(wr::Writer & w) const w.Write(m_title); w.Write(m_url); w.Write(m_thumbnailUrl); - w.Write(m_parentUrl); + w.Write(m_parentIndex); WriteCoord(w, m_lat); WriteCoord(w, m_lon); @@ -49,7 +49,7 @@ void ArticleInfo::Read(rd::Reader & r) r.Read(m_title); r.Read(m_url); r.Read(m_thumbnailUrl); - r.Read(m_parentUrl); + r.Read(m_parentIndex); ReadCoord(r, m_lat); ReadCoord(r, m_lon); @@ -68,10 +68,10 @@ double ArticleInfo::Score(double currLat, double currLon) const void ArticleInfo::Swap(ArticleInfo & i) { m_key.swap(i.m_key); - m_url.swap(i.m_url); m_title.swap(i.m_title); + m_url.swap(i.m_url); m_thumbnailUrl.swap(i.m_thumbnailUrl); - m_parentUrl.swap(i.m_parentUrl); + std::swap(m_parentIndex, i.m_parentIndex); std::swap(m_lat, i.m_lat); std::swap(m_lon, i.m_lon); } @@ -87,10 +87,10 @@ namespace bool ArticleInfo::operator == (ArticleInfo const & r) const { return (m_key == r.m_key && - m_url == r.m_url && m_title == r.m_title && + m_url == r.m_url && m_thumbnailUrl == r.m_thumbnailUrl && - m_parentUrl == r.m_parentUrl && + m_parentIndex == r.m_parentIndex && EqualCoord(m_lat, r.m_lat) && EqualCoord(m_lon, r.m_lon)); } diff --git a/storage/article_info.hpp b/storage/article_info.hpp index 8996aec..eaa5e75 100644 --- a/storage/article_info.hpp +++ b/storage/article_info.hpp @@ -12,21 +12,22 @@ const int EMPTY_COORD = 1000; class ArticleInfo { + string m_key; void GenerateKey(); - string m_key; - public: - ArticleInfo() {} - ArticleInfo(string const & title) : m_title(title) + ArticleInfo() : m_parentIndex(NO_PARENT) {} + ArticleInfo(string const & title) : m_title(title), m_parentIndex(NO_PARENT) { GenerateKey(); } - string m_url; + static const int32_t NO_PARENT = -1; + string m_title; + string m_url; string m_thumbnailUrl; - string m_parentUrl; + int32_t m_parentIndex; // NO_PARENT is the root article double m_lat, m_lon; diff --git a/storage/storage.cpp b/storage/storage.cpp index 46bdd0d..efe02c4 100644 --- a/storage/storage.cpp +++ b/storage/storage.cpp @@ -2,30 +2,11 @@ #include "../env/strings.hpp" #include "../env/reader.hpp" -#include "../env/writer.hpp" -#include "../env/logging.hpp" #include "../std/algorithm.hpp" #include "../std/utility.hpp" -void Storage::Add(ArticleInfo const & a) -{ - m_info.push_back(a); -} - -void Storage::Save(string const & path) -{ - SortByKey(); - - wr::FileWriter w(path); - size_t const count = m_info.size(); - w.Write(static_cast(count)); - - for (size_t i = 0; i < count; ++i) - m_info[i].Write(w); -} - void Storage::Load(string const & path) { rd::SequenceFileReader reader(path); @@ -52,48 +33,3 @@ void Storage::QueryArticleInfos(vector & out, string const & prefix out.assign(range.first, range.second); sort(out.begin(), out.end(), ArticleInfo::LessScore(lat, lon)); } - -void Storage::SortByKey() -{ - sort(m_info.begin(), m_info.end(), ArticleInfo::LessStorage()); -} - - -StorageMock::StorageMock() -{ - ArticleInfo i1("London"); - i1.m_url = "London.html"; - i1.m_thumbnailUrl = "london.jpg"; - i1.m_parentUrl = "Europe -> Great Britain"; - i1.m_lat = 51.50726; - i1.m_lon = -0.12765; - m_info.push_back(i1); - - ArticleInfo i2("Lancaster"); - i2.m_url = "Lancaster.html"; - i2.m_thumbnailUrl = "lancaster.jpg"; - i2.m_parentUrl = "Europe -> Great Britain"; - i2.m_lat = 54.04839; - i2.m_lon = -2.79904; - m_info.push_back(i2); - - ArticleInfo i3("Great Britain"); - i3.m_url = "GreatBritain.html"; - i3.m_thumbnailUrl = "great_britain.jpg"; - i3.m_parentUrl = "Europe"; - i3.m_lat = 54.70235; - i3.m_lon = -3.27656; - m_info.push_back(i3); - - SortByKey(); -} - -bool StorageMock::operator == (StorageMock const & r) const -{ - if (r.m_info.size() != m_info.size()) - return false; - for (size_t i = 0; i < m_info.size();++i) - if (!(m_info[i] == r.m_info[i])) - return false; - return true; -} diff --git a/storage/storage.hpp b/storage/storage.hpp index 401c7c3..7db5845 100644 --- a/storage/storage.hpp +++ b/storage/storage.hpp @@ -9,28 +9,27 @@ class Storage { public: - /// @name Used in generator. - //@{ - void Add(ArticleInfo const & a); - void Save(string const & path); - //@} - void Load(string const & path); void QueryArticleInfos(vector & out, string const & prefix, double lat = EMPTY_COORD, double lon = EMPTY_COORD) const; + /// For unit tests only. + ArticleInfo const & GetArticle(size_t i) const { return m_info[i]; } + + ArticleInfo const * GetParentArticle(ArticleInfo const & info) const + { + return (info.m_parentIndex != ArticleInfo::NO_PARENT ? &m_info[info.m_parentIndex] : 0); + } + + string GetParentName(ArticleInfo const & info) const + { + ArticleInfo const * p = GetParentArticle(info); + return (p ? p->m_title : string()); + } + protected: - void SortByKey(); - vector m_info; -}; - - -class StorageMock : public Storage -{ -public: - StorageMock(); - - bool operator == (StorageMock const & r) const; + + friend class StorageBuilder; }; diff --git a/storage/storage.pro b/storage/storage.pro index 6b51d39..6ef31d2 100644 --- a/storage/storage.pro +++ b/storage/storage.pro @@ -9,11 +9,13 @@ HEADERS += \ article_info.hpp \ distance.hpp \ storage.hpp \ + storage_builder.hpp \ SOURCES += \ article_info.cpp \ distance.cpp \ storage.cpp \ + storage_builder.cpp \ # env sources SOURCES += \ diff --git a/storage/storage_builder.cpp b/storage/storage_builder.cpp new file mode 100644 index 0000000..7bbde8a --- /dev/null +++ b/storage/storage_builder.cpp @@ -0,0 +1,94 @@ +#include "storage_builder.hpp" + +#include "../env/writer.hpp" +#include "../env/assert.hpp" + + +void StorageBuilder::Add(ArticleInfoBuilder const & info) +{ + m_info.push_back(info); +} + +void StorageBuilder::ProcessArticles() +{ + sort(m_info.begin(), m_info.end(), ArticleInfo::LessStorage()); + + size_t const count = m_info.size(); + for (size_t i = 0; i < count; ++i) + { + size_t j = 0; + for (; j < count; ++j) + { + if (i != j && m_info[i].m_parentUrl == m_info[j].m_url) + { + m_info[i].m_parentIndex = j; + break; + } + } + } +} + +void StorageBuilder::Save(string const & path) +{ + ProcessArticles(); + + wr::FileWriter w(path); + size_t const count = m_info.size(); + w.Write(static_cast(count)); + + for (size_t i = 0; i < count; ++i) + m_info[i].Write(w); +} + +void StorageBuilder::Assign(Storage & storage) +{ + ProcessArticles(); + + storage.m_info.assign(m_info.begin(), m_info.end()); +} + +bool StorageBuilder::operator == (Storage const & s) const +{ + if (m_info.size() != s.m_info.size()) + return false; + + for (size_t i = 0; i < m_info.size();++i) + if (!(m_info[i] == s.m_info[i])) + return false; + + return true; +} + + +void InitStorageBuilderMock(StorageBuilder & builder) +{ + ArticleInfoBuilder i1("London"); + i1.m_url = "London"; + i1.m_thumbnailUrl = "london.jpg"; + i1.m_parentUrl = "Great_Britain"; + i1.m_lat = 51.50726; + i1.m_lon = -0.12765; + builder.Add(i1); + + ArticleInfoBuilder i2("Lancaster"); + i2.m_url = "Lancaster"; + i2.m_thumbnailUrl = "lancaster.jpg"; + i2.m_parentUrl = "Great_Britain"; + i2.m_lat = 54.04839; + i2.m_lon = -2.79904; + builder.Add(i2); + + ArticleInfoBuilder i3("Great Britain"); + i3.m_url = "Great_Britain"; + i3.m_thumbnailUrl = "great_britain.jpg"; + i3.m_lat = 54.70235; + i3.m_lon = -3.27656; + builder.Add(i3); +} + +StorageMock::StorageMock() +{ + StorageBuilder builder; + InitStorageBuilderMock(builder); + builder.Assign(*this); +} diff --git a/storage/storage_builder.hpp b/storage/storage_builder.hpp new file mode 100644 index 0000000..e13c585 --- /dev/null +++ b/storage/storage_builder.hpp @@ -0,0 +1,49 @@ +#pragma once + +#include "storage.hpp" +#include "article_info.hpp" + + +class ArticleInfoBuilder : public ArticleInfo +{ +public: + ArticleInfoBuilder(string const & title) : ArticleInfo(title) {} + + string m_parentUrl; + + void Swap(ArticleInfoBuilder & b) + { + m_parentUrl.swap(b.m_parentUrl); + ArticleInfo::Swap(b); + } +}; + +inline void swap(ArticleInfoBuilder & b1, ArticleInfoBuilder & b2) +{ + b1.Swap(b2); +} + + +class StorageBuilder +{ + vector m_info; + + void ProcessArticles(); + +public: + void Add(ArticleInfoBuilder const & info); + + void Save(string const & path); + + void Assign(Storage & storage); + + bool operator == (Storage const & s) const; +}; + +void InitStorageBuilderMock(StorageBuilder & builder); + +class StorageMock : public Storage +{ +public: + StorageMock(); +}; diff --git a/storage/tests/storage_test.cpp b/storage/tests/storage_test.cpp index 150970c..73d1f51 100644 --- a/storage/tests/storage_test.cpp +++ b/storage/tests/storage_test.cpp @@ -1,6 +1,6 @@ #include -#include "storage.hpp" +#include "storage_builder.hpp" #include "distance.hpp" #include "../env/message_std.hpp" @@ -32,7 +32,7 @@ public: for (size_t i = 0; i < size; ++i) m_info.push_back(ArticleInfo(arr[i])); - SortByKey(); + sort(m_info.begin(), m_info.end(), ArticleInfo::LessStorage()); } void CheckBounds(string const & beg, string const & end) const @@ -43,7 +43,7 @@ public: } -TEST(Storage, PrefixQuery1) +TEST(Storage, PrefixQuery) { StorageTest storage; @@ -77,21 +77,6 @@ TEST(Storage, PrefixQuery1) EXPECT_EQ(out.size(), 0); } -TEST(Storage, PrefixQuery2) -{ - StorageTest storage; - - char const * arrTitle[] = { "London", "Lancaster", "Great Britan" }; - size_t const count = ArraySize(arrTitle); - - storage.FillStorage(arrTitle, count); - - vector out; - storage.QueryArticleInfos(out, "l"); - EXPECT_EQ(out.size(), 2); - CheckBounds(out, "Lancaster", "London"); -} - TEST(Storage, PrefixQuery_Utf8) { StorageTest storage; @@ -145,13 +130,12 @@ TEST(Storage, PrefixQuery_lowerCaseTest) TEST(Storage, ArticleInfoRW) { ArticleInfo i("Über Karten"); - i.m_url = "Great_Britain"; + i.m_url = "Éařízení"; i.m_thumbnailUrl = "great_britain.jpg"; - i.m_parentUrl = "Schließen"; i.m_lat = 5.67894; i.m_lon = 89.12345; - char const * name = "file.dat"; + char const * name = "article.dat"; { wr::FileWriter w(name); i.Write(w); @@ -167,25 +151,32 @@ TEST(Storage, ArticleInfoRW) fs::DeleteFile(name); } -TEST(Storage, StorageReadWriteTest) +TEST(Storage, StorageBuilderRW) { - StorageMock s1; + StorageBuilder builder; + InitStorageBuilderMock(builder); - ArticleInfo i("Über Karten"); - i.m_url = "Great_Britain"; + ArticleInfoBuilder i("Über Karten"); + i.m_url = "Éařízení"; i.m_thumbnailUrl = "great_britain.jpg"; i.m_parentUrl = "Schließen"; i.m_lat = 5.67894; i.m_lon = 89.12345; - s1.Add(i); + builder.Add(i); - char const * name = "storage_mock.dat"; - s1.Save(name); + char const * name = "storage.dat"; + builder.Save(name); - StorageMock s2; - s2.Load(name); + Storage storage; + storage.Load(name); - EXPECT_EQ(s1, s2); + EXPECT_EQ(builder, storage); + + ArticleInfo const * zero = 0; + EXPECT_EQ(storage.GetParentArticle(storage.GetArticle(0)), zero); + EXPECT_NE(storage.GetParentArticle(storage.GetArticle(1)), zero); + EXPECT_NE(storage.GetParentArticle(storage.GetArticle(2)), zero); + EXPECT_EQ(storage.GetParentArticle(storage.GetArticle(3)), zero); fs::DeleteFile(name); }