From c6aaf72e4effeaae4102c41e25dcce162399f339 Mon Sep 17 00:00:00 2001 From: Maxim Pimenov Date: Fri, 16 Mar 2018 19:44:18 +0300 Subject: [PATCH] [search] Replaced MaybeRelevance with boost::optional. --- platform/platform.hpp | 10 ++-- .../assessment_tool/context.cpp | 12 ++--- .../search_quality/assessment_tool/edits.cpp | 23 ++++------ .../search_quality/assessment_tool/edits.hpp | 46 ++++--------------- .../assessment_tool/main_model.cpp | 9 ++-- .../assessment_tool/main_model.hpp | 4 +- .../assessment_tool/result_view.cpp | 14 +++--- 7 files changed, 43 insertions(+), 75 deletions(-) diff --git a/platform/platform.hpp b/platform/platform.hpp index 1635eecb5a..a65420ab66 100644 --- a/platform/platform.hpp +++ b/platform/platform.hpp @@ -283,13 +283,13 @@ public: switch (thread) { case Thread::File: - m_fileThread->Push(forward(task)); + m_fileThread->Push(std::forward(task)); break; case Thread::Network: - m_networkThread->Push(forward(task)); + m_networkThread->Push(std::forward(task)); break; case Thread::Gui: - RunOnGuiThread(forward(task)); + RunOnGuiThread(std::forward(task)); break; } } @@ -301,10 +301,10 @@ public: switch (thread) { case Thread::File: - m_fileThread->PushDelayed(delay, forward(task)); + m_fileThread->PushDelayed(delay, std::forward(task)); break; case Thread::Network: - m_networkThread->PushDelayed(delay, forward(task)); + m_networkThread->PushDelayed(delay, std::forward(task)); break; case Thread::Gui: CHECK(false, ("Delayed tasks for gui thread are not supported yet")); diff --git a/search/search_quality/assessment_tool/context.cpp b/search/search_quality/assessment_tool/context.cpp index caf82c0d42..9507febd28 100644 --- a/search/search_quality/assessment_tool/context.cpp +++ b/search/search_quality/assessment_tool/context.cpp @@ -54,20 +54,20 @@ search::Sample Context::MakeSample(search::FeatureLoader & loader) const auto const & entry = nonFoundEntries[k++]; auto const deleted = entry.m_deleted; auto const & curr = entry.m_curr; - if (!deleted && !curr.m_unknown) + if (!deleted && curr) { auto result = m_sample.m_results[i]; - result.m_relevance = curr.m_relevance; + result.m_relevance = *curr; outResults.push_back(result); } continue; } - if (foundEntries[j].m_curr.m_unknown) + if (!foundEntries[j].m_curr) continue; auto result = m_sample.m_results[i]; - result.m_relevance = foundEntries[j].m_curr.m_relevance; + result.m_relevance = *foundEntries[j].m_curr; outResults.push_back(move(result)); } @@ -81,7 +81,7 @@ search::Sample Context::MakeSample(search::FeatureLoader & loader) const continue; } - if (foundEntries[i].m_curr.m_unknown) + if (!foundEntries[i].m_curr) continue; auto const & result = m_foundResults[i]; @@ -91,7 +91,7 @@ search::Sample Context::MakeSample(search::FeatureLoader & loader) const FeatureType ft; CHECK(loader.Load(result.GetFeatureID(), ft), ()); - outResults.push_back(search::Sample::Result::Build(ft, foundEntries[i].m_curr.m_relevance)); + outResults.push_back(search::Sample::Result::Build(ft, *foundEntries[i].m_curr)); } return outSample; diff --git a/search/search_quality/assessment_tool/edits.cpp b/search/search_quality/assessment_tool/edits.cpp index 47dbfcad11..8a0dfdfc53 100644 --- a/search/search_quality/assessment_tool/edits.cpp +++ b/search/search_quality/assessment_tool/edits.cpp @@ -4,7 +4,7 @@ namespace { -void UpdateNumEdits(Edits::Entry const & entry, Edits::MaybeRelevance const & r, size_t & numEdits) +void UpdateNumEdits(Edits::Entry const & entry, Edits::Relevance const & r, size_t & numEdits) { if (entry.m_curr != entry.m_orig && r == entry.m_orig) { @@ -27,7 +27,7 @@ bool Edits::Editor::Set(Relevance relevance) return m_parent.SetRelevance(m_index, relevance); } -Edits::MaybeRelevance Edits::Editor::Get() const +boost::optional Edits::Editor::Get() const { return m_parent.Get(m_index).m_curr; } @@ -52,7 +52,7 @@ void Edits::Apply() }); } -void Edits::Reset(std::vector const & relevances) +void Edits::Reset(std::vector> const & relevances) { WithObserver(Update::MakeAll(), [this, &relevances]() { m_entries.resize(relevances.size()); @@ -75,11 +75,9 @@ bool Edits::SetRelevance(size_t index, Relevance relevance) auto & entry = m_entries[index]; - MaybeRelevance const r(relevance); + UpdateNumEdits(entry, relevance, m_numEdits); - UpdateNumEdits(entry, r, m_numEdits); - - entry.m_curr = r; + entry.m_curr = relevance; return entry.m_curr != entry.m_orig; }); } @@ -89,11 +87,8 @@ void Edits::SetAllRelevances(Relevance relevance) WithObserver(Update::MakeAll(), [this, relevance]() { for (auto & entry : m_entries) { - MaybeRelevance const r(relevance); - - UpdateNumEdits(entry, r, m_numEdits); - - entry.m_curr = r; + UpdateNumEdits(entry, relevance, m_numEdits); + entry.m_curr = relevance; } }); } @@ -152,9 +147,9 @@ Edits::Entry const & Edits::GetEntry(size_t index) const return m_entries[index]; } -std::vector Edits::GetRelevances() const +std::vector> Edits::GetRelevances() const { - std::vector relevances(m_entries.size()); + std::vector> relevances(m_entries.size()); for (size_t i = 0; i < m_entries.size(); ++i) relevances[i] = m_entries[i].m_curr; return relevances; diff --git a/search/search_quality/assessment_tool/edits.hpp b/search/search_quality/assessment_tool/edits.hpp index b73e24f743..5c94bbe80c 100644 --- a/search/search_quality/assessment_tool/edits.hpp +++ b/search/search_quality/assessment_tool/edits.hpp @@ -10,42 +10,13 @@ #include #include +#include + class Edits { public: using Relevance = search::Sample::Result::Relevance; - struct MaybeRelevance - { - MaybeRelevance() = default; - MaybeRelevance(search::Sample::Result::Relevance relevance) - : m_relevance(relevance), m_unknown(false) - { - } - - bool operator==(MaybeRelevance const & rhs) const - { - if (m_unknown && rhs.m_unknown) - return true; - if (m_unknown != rhs.m_unknown) - return false; - ASSERT(!m_unknown, ()); - ASSERT(!rhs.m_unknown, ()); - return m_relevance == rhs.m_relevance; - } - - bool operator!=(MaybeRelevance const & rhs) const { return !(*this == rhs); } - - Relevance m_relevance = Relevance::Irrelevant; - // The guard for |m_relevance|. The |m_relevance| field - // should be read only when |m_unknown| is false. - // It is implemented as a separate guard instead of another value - // in the Relevance's enum because it results in cleaner typing - // in our case (we only use unknown in the UI, so serializing - // it makes no sense). - bool m_unknown = true; - }; - struct Entry { enum class Type @@ -55,12 +26,13 @@ public: }; Entry() = default; - Entry(MaybeRelevance relevance, Type type) : m_curr(relevance), m_orig(relevance), m_type(type) + Entry(boost::optional relevance, Type type) + : m_curr(relevance), m_orig(relevance), m_type(type) { } - MaybeRelevance m_curr = {}; - MaybeRelevance m_orig = {}; + boost::optional m_curr = {}; + boost::optional m_orig = {}; bool m_deleted = false; Type m_type = Type::Loaded; }; @@ -101,7 +73,7 @@ public: // Sets relevance to |relevance|. Returns true iff |relevance| // differs from the original one. bool Set(Relevance relevance); - MaybeRelevance Get() const; + boost::optional Get() const; bool HasChanges() const; Entry::Type GetType() const; @@ -113,7 +85,7 @@ public: explicit Edits(OnUpdate onUpdate) : m_onUpdate(onUpdate) {} void Apply(); - void Reset(std::vector const & relevances); + void Reset(std::vector> const & relevances); // Sets relevance at |index| to |relevance|. Returns true iff // |relevance| differs from the original one. @@ -135,7 +107,7 @@ public: Entry & GetEntry(size_t index); Entry const & GetEntry(size_t index) const; size_t NumEntries() const { return m_entries.size(); } - std::vector GetRelevances() const; + std::vector> GetRelevances() const; Entry const & Get(size_t index) const; diff --git a/search/search_quality/assessment_tool/main_model.cpp b/search/search_quality/assessment_tool/main_model.cpp index a8f22b4e91..a749b1a708 100644 --- a/search/search_quality/assessment_tool/main_model.cpp +++ b/search/search_quality/assessment_tool/main_model.cpp @@ -23,7 +23,6 @@ #include #include -using Relevance = search::Sample::Result::Relevance; using namespace std; // MainModel::SampleContext ------------------------------------------------------------------------ @@ -141,7 +140,7 @@ void MainModel::OnSampleSelected(int index) search::SearchParams params; sample.FillSearchParams(params); params.m_onResults = [this, index, sample, timestamp](search::Results const & results) { - vector relevances; + vector> relevances; vector goldenMatching; vector actualMatching; @@ -160,7 +159,7 @@ void MainModel::OnSampleSelected(int index) if (j != search::Matcher::kInvalidId) { CHECK_LESS(j, relevances.size(), ()); - relevances[j] = Edits::MaybeRelevance(sample.m_results[i].m_relevance); + relevances[j] = sample.m_results[i].m_relevance; } } } @@ -322,7 +321,7 @@ void MainModel::OnUpdate(View::ResultType type, size_t sampleIndex, Edits::Updat } void MainModel::OnResults(uint64_t timestamp, size_t sampleIndex, search::Results const & results, - vector const & relevances, + vector> const & relevances, vector const & goldenMatching, vector const & actualMatching) { @@ -348,7 +347,7 @@ void MainModel::OnResults(uint64_t timestamp, size_t sampleIndex, search::Result context.m_actualMatching = actualMatching; { - vector relevances; + vector> relevances; auto & nonFound = context.m_nonFoundResults; CHECK(nonFound.empty(), ()); diff --git a/search/search_quality/assessment_tool/main_model.hpp b/search/search_quality/assessment_tool/main_model.hpp index 1909f6c989..32fd12ea82 100644 --- a/search/search_quality/assessment_tool/main_model.hpp +++ b/search/search_quality/assessment_tool/main_model.hpp @@ -14,6 +14,8 @@ #include #include +#include + class Framework; class Index; @@ -49,7 +51,7 @@ private: void OnUpdate(View::ResultType type, size_t sampleIndex, Edits::Update const & update); void OnResults(uint64_t timestamp, size_t sampleIndex, search::Results const & results, - std::vector const & relevances, + std::vector> const & relevances, std::vector const & goldenMatching, std::vector const & actualMatching); diff --git a/search/search_quality/assessment_tool/result_view.cpp b/search/search_quality/assessment_tool/result_view.cpp index 505d61a309..bef6d3ad5f 100644 --- a/search/search_quality/assessment_tool/result_view.cpp +++ b/search/search_quality/assessment_tool/result_view.cpp @@ -172,13 +172,13 @@ void ResultView::UpdateRelevanceRadioButtons() m_vital->setChecked(false); auto const & r = m_editor->Get(); - if (!r.m_unknown) + if (!r) + return; + + switch (*r) { - switch (r.m_relevance) - { - case Relevance::Irrelevant: m_irrelevant->setChecked(true); break; - case Relevance::Relevant: m_relevant->setChecked(true); break; - case Relevance::Vital: m_vital->setChecked(true); break; - } + case Relevance::Irrelevant: m_irrelevant->setChecked(true); break; + case Relevance::Relevant: m_relevant->setChecked(true); break; + case Relevance::Vital: m_vital->setChecked(true); break; } }