Simpler options checks: Either page and per_page, or limit and offset

kfa-wtag · Dec 4, 2015 · 1e698b0 · 1e698b0
1 parent 36ea472
commit 1e698b0
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 18 deletions.
diff --git a/lib/xapian_db/resultset.rb b/lib/xapian_db/resultset.rb
@@ -63,17 +63,17 @@ def initialize(enquiry, options={})
       raise ArgumentError.new "unsupported options for resultset: #{params}" if params.size > 0
       raise ArgumentError.new "db_size option is required" unless db_size
 
-      raise ArgumentError.new "impossible combination of parameters" unless per_page.nil? || limit.nil? || per_page == limit
+      unless (page.nil? && per_page.nil?) || (limit.nil? && offset.nil?)
+        raise ArgumentError.new "Impossible combination of parameters. Either pass page and/or per_page, or limit and/or offset."
+      end
 
       calculated_page = offset.nil? || limit.nil? ? nil : (offset.to_f / limit.to_f) + 1
-      limit           = limit.nil? ? db_size : limit.to_i
-      per_page        = per_page.nil? ? limit.to_i : per_page.to_i
 
-      raise ArgumentError.new "impossible combination of parameters" unless offset.nil? || page.nil? || ((page - 1) * per_page) == offset
-
-      page   = page.nil? ? (calculated_page.nil? ? 1 : calculated_page) : page.to_i
-      offset = offset.nil? ? (page - 1) * per_page : offset.to_i
-      count  = per_page < limit ? per_page : limit
+      limit    = limit.nil? ? db_size : limit.to_i
+      per_page = per_page.nil? ? limit.to_i : per_page.to_i
+      page     = page.nil? ? (calculated_page.nil? ? 1 : calculated_page) : page.to_i
+      offset   = offset.nil? ? (page - 1) * per_page : offset.to_i
+      count    = per_page < limit ? per_page : limit
 
       return build_empty_resultset if (page - 1) * per_page > db_size
       result_window = enquiry.mset(offset, count)

diff --git a/spec/xapian_db/resultset_spec.rb b/spec/xapian_db/resultset_spec.rb
@@ -125,16 +125,9 @@
       expect(XapianDb::Resultset.new(@enquiry, db_size: @matches.size, per_page: 2, :page => 3).count).to eq(0)
     end
 
-    it "raises an exception if incompatible elements of both per_page and limit are given" do
-      expect{XapianDb::Resultset.new(@enquiry, db_size: @matches.size, per_page: 2, limit: 3)}.to raise_error ArgumentError, "impossible combination of parameters"
-    end
-
-    it "raises an exception if incompatible elements of both page / per_page and offset / limit are given" do
-      expect{XapianDb::Resultset.new(@enquiry, db_size: @matches.size, per_page: 2, page: 2, offset: 1, limit: 2)}.to raise_error ArgumentError, "impossible combination of parameters"
-    end
-
-    it "raises no exception if compatible elements of both page / per_page and offset / limit are given" do
-      expect{XapianDb::Resultset.new(@enquiry, db_size: @matches.size, per_page: 2, page: 2, offset: 2, limit: 2)}.not_to raise_error
+    it "raises an exception if elements of both page / per_page and offset / limit are given" do
+      expected_error = "Impossible combination of parameters. Either pass page and/or per_page, or limit and/or offset."
+      expect{XapianDb::Resultset.new(@enquiry, db_size: @matches.size, per_page: 2, page: 2, offset: 1, limit: 2)}.to raise_error ArgumentError, expected_error
     end
 
     it "should populate itself with found xapian documents" do