Merge pull request #4492 from ygorshenin/jni-hotels-filter

[search] JNI layer for HotelsFilter.

.clang-format

@@ -22,3 +22,9 @@ NamespaceIndentation: None
 PointerAlignment: Middle
 SortIncludes: true
 Standard: Cpp11
+
+---
+Language: Java
+BreakAfterJavaFieldAnnotations: true
+BreakBeforeBraces: Allman
+ColumnLimit: 100

android/jni/com/mapswithme/maps/SearchEngine.cpp

@@ -1,10 +1,13 @@
 #include "Framework.hpp"
 
 #include "search/everywhere_search_params.hpp"
+#include "search/hotels_filter.hpp"
 #include "search/mode.hpp"
 #include "search/result.hpp"
 #include "search/viewport_search_params.hpp"
 
+#include "base/logging.hpp"
+
 #include "std/cstdint.hpp"
 
 #include "../core/jni_helper.hpp"
@@ -16,6 +19,185 @@ using search::Result;
 
 namespace
 {
+class HotelsFilterBuilder
+{
+public:
+  using Rule = shared_ptr<search::hotels_filter::Rule>;
+
+  // *NOTE* keep this in sync with Java counterpart.
+  enum Type
+  {
+    TYPE_AND = 0,
+    TYPE_OR = 1,
+    TYPE_OP = 2
+  };
+
+  // *NOTE* keep this in sync with Java counterpart.
+  enum Field
+  {
+    FIELD_RATING = 0,
+    FIELD_PRICE_RATE = 1
+  };
+
+  // *NOTE* keep this in sync with Java counterpart.
+  enum Op
+  {
+    OP_LT = 0,
+    OP_LE = 1,
+    OP_GT = 2,
+    OP_GE = 3,
+    OP_EQ = 4
+  };
+
+  void Init(JNIEnv * env)
+  {
+    if (m_initialized)
+      return;
+
+    {
+      auto const baseClass = env->FindClass("com/mapswithme/maps/search/HotelsFilter");
+      m_type = env->GetFieldID(baseClass, "mType", "I");
+    }
+
+    {
+      auto const andClass = env->FindClass("com/mapswithme/maps/search/HotelsFilter$And");
+      m_andLhs = env->GetFieldID(andClass, "mLhs", "Lcom/mapswithme/maps/search/HotelsFilter;");
+      m_andRhs = env->GetFieldID(andClass, "mRhs", "Lcom/mapswithme/maps/search/HotelsFilter;");
+    }
+
+    {
+      auto const orClass = env->FindClass("com/mapswithme/maps/search/HotelsFilter$Or");
+      m_orLhs = env->GetFieldID(orClass, "mLhs", "Lcom/mapswithme/maps/search/HotelsFilter;");
+      m_orRhs = env->GetFieldID(orClass, "mRhs", "Lcom/mapswithme/maps/search/HotelsFilter;");
+    }
+
+    {
+      auto const opClass = env->FindClass("com/mapswithme/maps/search/HotelsFilter$Op");
+      m_field = env->GetFieldID(opClass, "mField", "I");
+      m_op = env->GetFieldID(opClass, "mOp", "I");
+    }
+
+    {
+      auto const ratingFilterClass =
+          env->FindClass("com/mapswithme/maps/search/HotelsFilter$RatingFilter");
+      m_rating = env->GetFieldID(ratingFilterClass, "mValue", "F");
+    }
+
+    {
+      auto const priceRateFilterClass =
+          env->FindClass("com/mapswithme/maps/search/HotelsFilter$PriceRateFilter");
+      m_priceRate = env->GetFieldID(priceRateFilterClass, "mValue", "I");
+    }
+
+    m_initialized = true;
+  }
+
+  Rule Build(JNIEnv * env, jobject filter)
+  {
+    if (!m_initialized)
+      return {};
+
+    if (!filter)
+      return {};
+
+    auto const type = static_cast<int>(env->GetIntField(filter, m_type));
+
+    switch (type)
+    {
+    case TYPE_AND: return BuildAnd(env, filter);
+    case TYPE_OR: return BuildOr(env, filter);
+    case TYPE_OP: return BuildOp(env, filter);
+    }
+
+    LOG(LERROR, ("Unknown type:", type));
+    return {};
+  }
+
+private:
+  Rule BuildAnd(JNIEnv * env, jobject filter)
+  {
+    auto const lhs = env->GetObjectField(filter, m_andLhs);
+    auto const rhs = env->GetObjectField(filter, m_andRhs);
+    return search::hotels_filter::And(Build(env, lhs), Build(env, rhs));
+  }
+
+  Rule BuildOr(JNIEnv * env, jobject filter)
+  {
+    auto const lhs = env->GetObjectField(filter, m_orLhs);
+    auto const rhs = env->GetObjectField(filter, m_orRhs);
+    return search::hotels_filter::Or(Build(env, lhs), Build(env, rhs));
+  }
+
+  Rule BuildOp(JNIEnv * env, jobject filter)
+  {
+    auto const field = static_cast<int>(env->GetIntField(filter, m_field));
+    auto const op = static_cast<int>(env->GetIntField(filter, m_op));
+
+    switch (field)
+    {
+    case FIELD_RATING: return BuildRatingOp(env, op, filter);
+    case FIELD_PRICE_RATE: return BuildPriceRateOp(env, op, filter);
+    }
+
+    LOG(LERROR, ("Unknown field:", field));
+    return {};
+  }
+
+  Rule BuildRatingOp(JNIEnv * env, int op, jobject filter)
+  {
+    using namespace search::hotels_filter;
+
+    auto const rating = static_cast<float>(env->GetFloatField(filter, m_rating));
+
+    switch (op)
+    {
+    case OP_LT: return Lt<Rating>(rating);
+    case OP_LE: return Le<Rating>(rating);
+    case OP_GT: return Gt<Rating>(rating);
+    case OP_GE: return Ge<Rating>(rating);
+    case OP_EQ: return Eq<Rating>(rating);
+    }
+
+    LOG(LERROR, ("Unknown op:", op));
+    return {};
+  }
+
+  Rule BuildPriceRateOp(JNIEnv * env, int op, jobject filter)
+  {
+    using namespace search::hotels_filter;
+
+    auto const priceRate = static_cast<int>(env->GetIntField(filter, m_priceRate));
+
+    switch (op)
+    {
+    case OP_LT: return Lt<PriceRate>(priceRate);
+    case OP_LE: return Le<PriceRate>(priceRate);
+    case OP_GT: return Gt<PriceRate>(priceRate);
+    case OP_GE: return Ge<PriceRate>(priceRate);
+    case OP_EQ: return Eq<PriceRate>(priceRate);
+    }
+
+    LOG(LERROR, ("Unknown op:", op));
+    return {};
+  }
+
+  jfieldID m_type;
+
+  jfieldID m_andLhs;
+  jfieldID m_andRhs;
+
+  jfieldID m_orLhs;
+  jfieldID m_orRhs;
+
+  jfieldID m_field;
+  jfieldID m_op;
+
+  jfieldID m_rating;
+  jfieldID m_priceRate;
+
+  bool m_initialized = false;
+} g_hotelsFilterBuilder;
+
 // TODO yunitsky
 // Do not cache search results here, after new search will be implemented.
 // Currently we cannot serialize FeatureID of search result properly.
@@ -180,16 +362,19 @@ extern "C"
     g_mapResultsMethod = jni::GetMethodID(env, g_javaListener, "onMapSearchResults", "([Lcom/mapswithme/maps/search/NativeMapSearchListener$Result;JZ)V");
     g_mapResultClass = jni::GetGlobalClassRef(env, "com/mapswithme/maps/search/NativeMapSearchListener$Result");
     g_mapResultCtor = jni::GetConstructorID(env, g_mapResultClass, "(Ljava/lang/String;Ljava/lang/String;)V");
+
+    g_hotelsFilterBuilder.Init(env);
   }
 
-  JNIEXPORT jboolean JNICALL
-  Java_com_mapswithme_maps_search_SearchEngine_nativeRunSearch(JNIEnv * env, jclass clazz, jbyteArray bytes, jstring lang,
-                                                               jlong timestamp, jboolean hasPosition, jdouble lat, jdouble lon)
+  JNIEXPORT jboolean JNICALL Java_com_mapswithme_maps_search_SearchEngine_nativeRunSearch(
+      JNIEnv * env, jclass clazz, jbyteArray bytes, jstring lang, jlong timestamp,
+      jboolean hasPosition, jdouble lat, jdouble lon, jobject hotelsFilter)
   {
     search::EverywhereSearchParams params;
     params.m_query = jni::ToNativeString(env, bytes);
     params.m_inputLocale = ReplaceDeprecatedLanguageCode(jni::ToNativeString(env, lang));
     params.m_onResults = bind(&OnResults, _1, timestamp, false, hasPosition, lat, lon);
+    params.m_hotelsFilter = g_hotelsFilterBuilder.Build(env, hotelsFilter);
 
     bool const searchStarted = g_framework->NativeFramework()->SearchEverywhere(params);
     if (searchStarted)
@@ -197,13 +382,14 @@ extern "C"
     return searchStarted;
   }
 
-  JNIEXPORT void JNICALL
-  Java_com_mapswithme_maps_search_SearchEngine_nativeRunInteractiveSearch(JNIEnv * env, jclass clazz, jbyteArray bytes,
-                                                                          jstring lang, jlong timestamp, jboolean isMapAndTable)
+  JNIEXPORT void JNICALL Java_com_mapswithme_maps_search_SearchEngine_nativeRunInteractiveSearch(
+      JNIEnv * env, jclass clazz, jbyteArray bytes, jstring lang, jlong timestamp,
+      jboolean isMapAndTable, jobject hotelsFilter)
   {
     search::ViewportSearchParams vparams;
     vparams.m_query = jni::ToNativeString(env, bytes);
     vparams.m_inputLocale = ReplaceDeprecatedLanguageCode(jni::ToNativeString(env, lang));
+    vparams.m_hotelsFilter = g_hotelsFilterBuilder.Build(env, hotelsFilter);
 
     g_framework->NativeFramework()->SearchInViewport(vparams);
 
@@ -214,13 +400,14 @@ extern "C"
       eparams.m_inputLocale = vparams.m_inputLocale;
       eparams.m_onResults = bind(&OnResults, _1, timestamp, isMapAndTable, false /* hasPosition */,
                                  0.0 /* lat */, 0.0 /* lon */);
+      eparams.m_hotelsFilter = vparams.m_hotelsFilter;
       if (g_framework->NativeFramework()->SearchEverywhere(eparams))
         g_queryTimestamp = timestamp;
     }
   }
 
-  JNIEXPORT void JNICALL
-  Java_com_mapswithme_maps_search_SearchEngine_nativeRunSearchMaps(JNIEnv * env, jclass clazz, jbyteArray bytes, jstring lang, jlong timestamp)
+  JNIEXPORT void JNICALL Java_com_mapswithme_maps_search_SearchEngine_nativeRunSearchMaps(
+      JNIEnv * env, jclass clazz, jbyteArray bytes, jstring lang, jlong timestamp)
   {
     storage::DownloaderSearchParams params;
     params.m_query = jni::ToNativeString(env, bytes);

android/src/com/mapswithme/maps/routing/SearchWheel.java

@@ -233,7 +233,7 @@ class SearchWheel implements View.OnClickListener
   private void startSearch(SearchOption searchOption)
   {
     mCurrentOption = searchOption;
-    SearchEngine.searchInteractive(searchOption.mSearchQuery, System.nanoTime(), false /* isMapAndTable */);
+    SearchEngine.searchInteractive(searchOption.mSearchQuery, System.nanoTime(), false /* isMapAndTable */, null /* hotelsFilter */);
     refreshSearchButtonImage();
 
     toggleSearchLayout();

android/src/com/mapswithme/maps/search/HotelsFilter.java

@@ -0,0 +1,94 @@
+package com.mapswithme.maps.search;
+
+import android.support.annotation.NonNull;
+
+public class HotelsFilter
+{
+  // *NOTE* keep this in sync with JNI counterpart.
+  public final static int TYPE_AND = 0;
+  public final static int TYPE_OR = 1;
+  public final static int TYPE_OP = 2;
+
+  public final int mType;
+
+  protected HotelsFilter(int type)
+  {
+    mType = type;
+  }
+
+  public static class And extends HotelsFilter
+  {
+    @NonNull
+    public final HotelsFilter mLhs;
+    @NonNull
+    public final HotelsFilter mRhs;
+
+    public And(@NonNull HotelsFilter lhs, @NonNull HotelsFilter rhs)
+    {
+      super(TYPE_AND);
+      mLhs = lhs;
+      mRhs = rhs;
+    }
+  }
+
+  public static class Or extends HotelsFilter
+  {
+    @NonNull
+    public final HotelsFilter mLhs;
+    @NonNull
+    public final HotelsFilter mRhs;
+
+    public Or(@NonNull HotelsFilter lhs, @NonNull HotelsFilter rhs)
+    {
+      super(TYPE_OR);
+      mLhs = lhs;
+      mRhs = rhs;
+    }
+  }
+
+  public static class Op extends HotelsFilter
+  {
+    // *NOTE* keep this in sync with JNI counterpart.
+    public final static int FIELD_RATING = 0;
+    public final static int FIELD_PRICE_RATE = 1;
+
+    // *NOTE* keep this in sync with JNI counterpart.
+    public final static int OP_LT = 0;
+    public final static int OP_LE = 1;
+    public final static int OP_GT = 2;
+    public final static int OP_GE = 3;
+    public final static int OP_EQ = 4;
+
+    public final int mField;
+    public final int mOp;
+
+    protected Op(int field, int op)
+    {
+      super(TYPE_OP);
+      mField = field;
+      mOp = op;
+    }
+  }
+
+  public static class RatingFilter extends Op
+  {
+    public final float mValue;
+
+    public RatingFilter(int op, float value)
+    {
+      super(FIELD_RATING, op);
+      mValue = value;
+    }
+  }
+
+  public static class PriceRateFilter extends Op
+  {
+    public final int mValue;
+
+    public PriceRateFilter(int op, int value)
+    {
+      super(FIELD_PRICE_RATE, op);
+      mValue = value;
+    }
+  }
+}

android/src/com/mapswithme/maps/search/SearchEngine.java

@@ -8,6 +8,8 @@ import com.mapswithme.util.Language;
 import com.mapswithme.util.Listeners;
 import com.mapswithme.util.concurrency.UiThread;
 
+import android.support.annotation.Nullable;
+
 public enum SearchEngine implements NativeSearchListener,
                                     NativeMapSearchListener
 {
@@ -95,21 +97,21 @@ public enum SearchEngine implements NativeSearchListener,
    * @param timestamp Search results are filtered according to it after multiple requests.
    * @return whether search was actually started.
    */
-  public static boolean search(String query, long timestamp, boolean hasLocation, double lat, double lon)
+  public static boolean search(String query, long timestamp, boolean hasLocation, double lat, double lon, @Nullable HotelsFilter hotelsFilter)
   {
     try
     {
-      return nativeRunSearch(query.getBytes("utf-8"), Language.getKeyboardLocale(), timestamp, hasLocation, lat, lon);
+      return nativeRunSearch(query.getBytes("utf-8"), Language.getKeyboardLocale(), timestamp, hasLocation, lat, lon, hotelsFilter);
     } catch (UnsupportedEncodingException ignored) { }
 
     return false;
   }
 
-  public static void searchInteractive(String query, long timestamp, boolean isMapAndTable)
+  public static void searchInteractive(String query, long timestamp, boolean isMapAndTable, @Nullable HotelsFilter hotelsFilter)
   {
     try
     {
-      nativeRunInteractiveSearch(query.getBytes("utf-8"), Language.getKeyboardLocale(), timestamp, isMapAndTable);
+      nativeRunInteractiveSearch(query.getBytes("utf-8"), Language.getKeyboardLocale(), timestamp, isMapAndTable, hotelsFilter);
     } catch (UnsupportedEncodingException ignored) { }
   }
 
@@ -154,12 +156,12 @@ public enum SearchEngine implements NativeSearchListener,
   /**
    * @param bytes utf-8 formatted bytes of query.
    */
-  private static native boolean nativeRunSearch(byte[] bytes, String language, long timestamp, boolean hasLocation, double lat, double lon);
+  private static native boolean nativeRunSearch(byte[] bytes, String language, long timestamp, boolean hasLocation, double lat, double lon, @Nullable HotelsFilter hotelsFilter);
 
   /**
    * @param bytes utf-8 formatted query bytes
    */
-  private static native void nativeRunInteractiveSearch(byte[] bytes, String language, long timestamp, boolean isMapAndTable);
+  private static native void nativeRunInteractiveSearch(byte[] bytes, String language, long timestamp, boolean isMapAndTable, @Nullable HotelsFilter hotelsFilter);
 
   /**
    * @param bytes utf-8 formatted query bytes

android/src/com/mapswithme/maps/search/SearchFragment.java

@@ -380,7 +380,10 @@ public class SearchFragment extends BaseMwmFragment
     final String query = getQuery();
     SearchRecents.add(query);
     mLastQueryTimestamp = System.nanoTime();
-    SearchEngine.searchInteractive(query, mLastQueryTimestamp, false /* isMapAndTable */);
+
+    // TODO (@alexzatsepin): set up hotelsFilter correctly.
+    SearchEngine.searchInteractive(
+        query, mLastQueryTimestamp, false /* isMapAndTable */, null /* hotelsFilter */);
     SearchEngine.showAllResults(query);
     Utils.navigateToParent(getActivity());
 
@@ -408,12 +411,17 @@ public class SearchFragment extends BaseMwmFragment
     // TODO @yunitsky Implement more elegant solution.
     if (getActivity() instanceof MwmActivity)
     {
-      SearchEngine.searchInteractive(getQuery(), mLastQueryTimestamp, true /* isMapAndTable */);
+      SearchEngine.searchInteractive(
+          getQuery(), mLastQueryTimestamp, true /* isMapAndTable */, null /* hotelsFilter */);
     }
     else
     {
-      if (!SearchEngine.search(getQuery(), mLastQueryTimestamp, mLastPosition.valid, mLastPosition.lat, mLastPosition.lon))
+      // TODO (@alexzatsepin): set up hotelsFilter correctly.
+      if (!SearchEngine.search(getQuery(), mLastQueryTimestamp, mLastPosition.valid,
+              mLastPosition.lat, mLastPosition.lon, null /* hotelsFilter */))
+      {
         return;
+      }
     }
 
     mSearchRunning = true;

map/framework.cpp

@@ -1361,6 +1361,9 @@ bool Framework::QueryMayBeSkipped(SearchIntent const & intent, search::SearchPar
   if (lastParams.IsValidPosition() != params.IsValidPosition())
     return false;
 
+  if (!search::hotels_filter::Rule::IsIdentical(lastParams.m_hotelsFilter, params.m_hotelsFilter))
+    return false;
+
   return true;
 }
 

search/hotels_filter.cpp

@@ -42,6 +42,23 @@ void Description::FromFeature(FeatureType & ft)
   }
 }
 
+// Rule --------------------------------------------------------------------------------------------
+// static
+bool Rule::IsIdentical(shared_ptr<Rule> const & lhs, shared_ptr<Rule> const & rhs)
+{
+  if (lhs && !rhs)
+    return false;
+  if (!lhs && rhs)
+    return false;
+
+  if (lhs && rhs && !lhs->IdenticalTo(*rhs))
+    return false;
+
+  return true;
+}
+
+string DebugPrint(Rule const & rule) { return rule.ToString(); }
+
 // HotelsFilter::ScopedFilter ----------------------------------------------------------------------
 HotelsFilter::ScopedFilter::ScopedFilter(MwmSet::MwmId const & mwmId,
                                          Descriptions const & descriptions, shared_ptr<Rule> rule)

search/hotels_filter.hpp

@@ -8,6 +8,7 @@
 
 #include "std/map.hpp"
 #include "std/shared_ptr.hpp"
+#include "std/string.hpp"
 #include "std/unique_ptr.hpp"
 #include "std/utility.hpp"
 #include "std/vector.hpp"
@@ -33,6 +34,8 @@ struct Rating
   {
     return d.m_rating;
   }
+
+  static char const * Name() { return "Rating"; }
 };
 
 struct PriceRate
@@ -50,6 +53,8 @@ struct PriceRate
   {
     return d.m_priceRate;
   }
+
+  static char const * Name() { return "PriceRate"; }
 };
 
 struct Description
@@ -63,11 +68,18 @@ struct Description
 struct Rule
 {
   virtual ~Rule() = default;
+
+  static bool IsIdentical(shared_ptr<Rule> const & lhs, shared_ptr<Rule> const & rhs);
+
   virtual bool Matches(Description const & d) const = 0;
+  virtual bool IdenticalTo(Rule const & rhs) const = 0;
+  virtual string ToString() const = 0;
 };
 
+string DebugPrint(Rule const & rule);
+
 template <typename Field>
-struct EqRule : public Rule
+struct EqRule final : public Rule
 {
   using Value = typename Field::Value;
 
@@ -79,11 +91,24 @@ struct EqRule : public Rule
     return Field::Eq(Field::Select(d), m_value);
   }
 
+  bool IdenticalTo(Rule const & rhs) const override
+  {
+    auto const * r = dynamic_cast<EqRule const *>(&rhs);
+    return r && Field::Eq(r->m_value, m_value);
+  }
+
+  string ToString() const override
+  {
+    ostringstream os;
+    os << "[ " << Field::Name() << " == " << m_value << " ]";
+    return os.str();
+  }
+
   Value const m_value;
 };
 
 template <typename Field>
-struct LtRule : public Rule
+struct LtRule final : public Rule
 {
   using Value = typename Field::Value;
 
@@ -95,11 +120,54 @@ struct LtRule : public Rule
     return Field::Lt(Field::Select(d), m_value);
   }
 
+  bool IdenticalTo(Rule const & rhs) const override
+  {
+    auto const * r = dynamic_cast<LtRule const *>(&rhs);
+    return r && Field::Eq(r->m_value, m_value);
+  }
+
+  string ToString() const override
+  {
+    ostringstream os;
+    os << "[ " << Field::Name() << " < " << m_value << " ]";
+    return os.str();
+  }
+
   Value const m_value;
 };
 
 template <typename Field>
-struct GtRule : public Rule
+struct LeRule final : public Rule
+{
+  using Value = typename Field::Value;
+
+  LeRule(Value value) : m_value(value) {}
+
+  // Rule overrides:
+  bool Matches(Description const & d) const override
+  {
+    auto const value = Field::Select(d);
+    return Field::Lt(value, m_value) || Field::Eq(value, m_value);
+  }
+
+  bool IdenticalTo(Rule const & rhs) const override
+  {
+    auto const * r = dynamic_cast<LeRule const *>(&rhs);
+    return r && Field::Eq(r->m_value, m_value);
+  }
+
+  string ToString() const override
+  {
+    ostringstream os;
+    os << "[ " << Field::Name() << " <= " << m_value << " ]";
+    return os.str();
+  }
+
+  Value const m_value;
+};
+
+template <typename Field>
+struct GtRule final : public Rule
 {
   using Value = typename Field::Value;
 
@@ -111,10 +179,53 @@ struct GtRule : public Rule
     return Field::Gt(Field::Select(d), m_value);
   }
 
+  bool IdenticalTo(Rule const & rhs) const override
+  {
+    auto const * r = dynamic_cast<GtRule const *>(&rhs);
+    return r && Field::Eq(r->m_value, m_value);
+  }
+
+  string ToString() const override
+  {
+    ostringstream os;
+    os << "[ " << Field::Name() << " > " << m_value << " ]";
+    return os.str();
+  }
+
   Value const m_value;
 };
 
-struct AndRule : public Rule
+template <typename Field>
+struct GeRule final : public Rule
+{
+  using Value = typename Field::Value;
+
+  GeRule(Value value) : m_value(value) {}
+
+  // Rule overrides:
+  bool Matches(Description const & d) const override
+  {
+    auto const value = Field::Select(d);
+    return Field::Gt(value, m_value) || Field::Eq(value, m_value);
+  }
+
+  bool IdenticalTo(Rule const & rhs) const override
+  {
+    auto const * r = dynamic_cast<GeRule const *>(&rhs);
+    return r && Field::Eq(r->m_value, m_value);
+  }
+
+  string ToString() const override
+  {
+    ostringstream os;
+    os << "[ " << Field::Name() << " >= " << m_value << " ]";
+    return os.str();
+  }
+
+  Value const m_value;
+};
+
+struct AndRule final : public Rule
 {
   AndRule(shared_ptr<Rule> lhs, shared_ptr<Rule> rhs) : m_lhs(move(lhs)), m_rhs(move(rhs)) {}
 
@@ -129,11 +240,28 @@ struct AndRule : public Rule
     return matches;
   }
 
+  bool IdenticalTo(Rule const & rhs) const override
+  {
+    auto const * r = dynamic_cast<AndRule const *>(&rhs);
+    return r && IsIdentical(m_lhs, r->m_lhs) && IsIdentical(m_rhs, r->m_rhs);
+  }
+
+  string ToString() const override
+  {
+    ostringstream os;
+    os << "[";
+    os << (m_lhs ? m_lhs->ToString() : "<none>");
+    os << " && ";
+    os << (m_rhs ? m_rhs->ToString() : "<none>");
+    os << "]";
+    return os.str();
+  }
+
   shared_ptr<Rule> m_lhs;
   shared_ptr<Rule> m_rhs;
 };
 
-struct OrRule : public Rule
+struct OrRule final : public Rule
 {
   OrRule(shared_ptr<Rule> lhs, shared_ptr<Rule> rhs) : m_lhs(move(lhs)), m_rhs(move(rhs)) {}
 
@@ -148,6 +276,23 @@ struct OrRule : public Rule
     return matches;
   }
 
+  bool IdenticalTo(Rule const & rhs) const override
+  {
+    auto const * r = dynamic_cast<OrRule const *>(&rhs);
+    return r && IsIdentical(m_lhs, r->m_lhs) && IsIdentical(m_rhs, r->m_rhs);
+  }
+
+  string ToString() const override
+  {
+    ostringstream os;
+    os << "[";
+    os << (m_lhs ? m_lhs->ToString() : "<none>");
+    os << " || ";
+    os << (m_rhs ? m_rhs->ToString() : "<none>");
+    os << "]";
+    return os.str();
+  }
+
   shared_ptr<Rule> m_lhs;
   shared_ptr<Rule> m_rhs;
 };
@@ -164,12 +309,24 @@ shared_ptr<Rule> Lt(typename Field::Value value)
   return make_shared<LtRule<Field>>(value);
 }
 
+template <typename Field>
+shared_ptr<Rule> Le(typename Field::Value value)
+{
+  return make_shared<LeRule<Field>>(value);
+}
+
 template <typename Field>
 inline shared_ptr<Rule> Gt(typename Field::Value value)
 {
   return make_shared<GtRule<Field>>(value);
 }
 
+template <typename Field>
+shared_ptr<Rule> Ge(typename Field::Value value)
+{
+  return make_shared<GeRule<Field>>(value);
+}
+
 inline shared_ptr<Rule> And(shared_ptr<Rule> lhs, shared_ptr<Rule> rhs)
 {
   return make_shared<AndRule>(lhs, rhs);
@@ -180,18 +337,6 @@ inline shared_ptr<Rule> Or(shared_ptr<Rule> lhs, shared_ptr<Rule> rhs)
   return make_shared<OrRule>(lhs, rhs);
 }
 
-template <typename Field>
-shared_ptr<Rule> Le(typename Field::Value value)
-{
-  return Or(Lt<Field>(value), Eq<Field>(value));
-}
-
-template <typename Field>
-shared_ptr<Rule> Ge(typename Field::Value value)
-{
-  return Or(Gt<Field>(value), Eq<Field>(value));
-}
-
 class HotelsFilter
 {
 public:

search/search_tests/hotels_filter_test.cpp

@@ -0,0 +1,35 @@
+#include "testing/testing.hpp"
+
+#include "search/hotels_filter.hpp"
+
+using namespace search::hotels_filter;
+
+namespace
+{
+UNIT_TEST(HotelsFilter_Identity)
+{
+  {
+    auto const first = And(Or(Eq<Rating>(5.0), Lt<PriceRate>(2)), Ge<Rating>(4.0));
+    auto const second = And(Or(Eq<Rating>(5.0), Lt<PriceRate>(2)), Ge<Rating>(4.0));
+    TEST(first.get(), ());
+    TEST(second.get(), ());
+    TEST(first->IdenticalTo(*second), (*first, *second));
+  }
+
+  {
+    auto const first = And(Gt<Rating>(5.0), Lt<PriceRate>(5));
+    auto const second = And(Lt<PriceRate>(5), Gt<Rating>(5.0));
+    TEST(first.get(), ());
+    TEST(second.get(), ());
+    TEST(!first->IdenticalTo(*second), (*first, *second));
+  }
+
+  {
+    auto const first = Ge<Rating>(1);
+    auto const second = Or(Gt<Rating>(1), Eq<Rating>(1));
+    TEST(first.get(), ());
+    TEST(second.get(), ());
+    TEST(!first->IdenticalTo(*second), (*first, *second));
+  }
+}
+}  // namespace

search/search_tests/search_tests.pro

@@ -19,6 +19,7 @@ macx-*: LIBS *= "-framework IOKit"
 SOURCES += \
     ../../testing/testingmain.cpp \
     algos_tests.cpp \
+    hotels_filter_test.cpp \
     house_detector_tests.cpp \
     house_numbers_matcher_test.cpp \
     interval_set_test.cpp \