Fixed one-off bounds calculation bug.

umpc · Jul 4, 2017 · 3c30513 · 3c30513
1 parent 7046817
commit 3c30513
Show file tree

Hide file tree

Showing 6 changed files with 68 additions and 54 deletions.
diff --git a/README.md b/README.md
@@ -53,11 +53,12 @@ func main() {
   iterCh, err := sm.BoundedIterCh(reversed, lowerBound, upperBound)
   if err != nil {
     fmt.Println(err)
-  }
-  defer iterCh.Close()
+  } else {
+    defer iterCh.Close()
 
-  for rec := range iterCh.Records() {
-    fmt.Printf("%+v\n", rec)
+    for rec := range iterCh.Records() {
+      fmt.Printf("%+v\n", rec)
+    }
   }
 }
 ```

diff --git a/bounds.go b/bounds.go
@@ -18,27 +18,43 @@ func (sm *SortedMap) setBoundIdx(boundVal interface{}) int {
 	if idx > 0 {
 		idx--
 	}
-	valFromIdx := sm.idx[sm.sorted[idx]]
-
-	// If the bound value is greater than or equal to the value from the map,
-	// select the next index value.
-	if idx < smLen-1 {
-		if sm.lessFn(valFromIdx, boundVal) {
-			idx++
-		}
-	}
 
 	return idx
 }
 
 func (sm *SortedMap) boundsIdxSearch(lowerBound, upperBound interface{}) []int {
+	smLen := len(sm.sorted)
+
+	if lowerBound != nil && upperBound != nil {
+		if sm.lessFn(upperBound, lowerBound) {
+			return nil
+		}
+	}
+
 	lowerBoundIdx := sm.setBoundIdx(lowerBound)
+	if lowerBound != nil {
+		if lowerBoundIdx < smLen-1 {
+			valFromIdx := sm.idx[sm.sorted[lowerBoundIdx]]
+
+			// If the bound value is greater than or equal to the value from the map,
+			// select the next index value.
+			if sm.lessFn(valFromIdx, lowerBound) {
+				lowerBoundIdx++
+			}
+		}
+	}
 
 	upperBoundIdx := 0
 	if upperBound == nil {
-		upperBoundIdx = len(sm.sorted) - 1
+		upperBoundIdx = smLen - 1
 	} else {
 		upperBoundIdx = sm.setBoundIdx(upperBound)
+		if upperBoundIdx < smLen-1 {
+			valFromIdx := sm.idx[sm.sorted[upperBoundIdx + 1]]
+			if !sm.lessFn(valFromIdx, upperBound) && !sm.lessFn(upperBound, valFromIdx) {
+				upperBoundIdx++
+			}
+		}
 	}
 
 	if lowerBound != nil && upperBound != nil {
@@ -47,7 +63,9 @@ func (sm *SortedMap) boundsIdxSearch(lowerBound, upperBound interface{}) []int {
 
 			if !sm.lessFn(lowerBound, upperBound) && !sm.lessFn(upperBound, lowerBound) {
 				// lowerBound == upperBound
-				return nil
+				if sm.lessFn(valFromIdx, lowerBound) || sm.lessFn(lowerBound, valFromIdx) {
+					return nil
+				}
 			}
 
 			if sm.lessFn(valFromIdx, lowerBound) || sm.lessFn(upperBound, valFromIdx) {

diff --git a/delete_test.go b/delete_test.go
@@ -105,7 +105,7 @@ func TestBoundedDelete(t *testing.T) {
 		}
 	}()
 
-	if err := sm.BoundedDelete(time.Now(), earlierDate); err != nil {
+	if err := sm.BoundedDelete(time.Now(), earlierDate); err == nil {
 		t.Fatal(err)
 	}
 
@@ -118,7 +118,7 @@ func TestBoundedDelete(t *testing.T) {
 		}
 	}()
 
-	if err := sm.BoundedDelete(earlierDate, earlierDate); err == nil {
+	if err := sm.BoundedDelete(earlierDate, earlierDate); err != nil {
 		t.Fatal(err)
 	}
 

diff --git a/examples/README.md b/examples/README.md
@@ -19,9 +19,9 @@
 The following function is used to generate test data in the examples:
 
 ```go
-func randRecords(n int) []*sortedmap.Record {
+func randRecords(n int) []sortedmap.Record {
   mrand.Seed(time.Now().UTC().UnixNano())
-  records := make([]*sortedmap.Record, n)
+  records := make([]sortedmap.Record, n)
   for i := range records {
     year := mrand.Intn(2058)
     for year < 2000 {
@@ -41,7 +41,7 @@ func randRecords(n int) []*sortedmap.Record {
 
     t := time.Date(year, mth, day, hour, min, sec, 0, time.UTC)
 
-    records[i] = &sortedmap.Record{
+    records[i] = sortedmap.Record{
       Key: t.Format(time.UnixDate),
       Val: t,
     }
@@ -174,11 +174,12 @@ func main() {
   iterCh, err := sm.BoundedIterCh(false, time.Time{}, time.Now())
   if err != nil {
     fmt.Println(err)
-  }
-  defer iterCh.Close()
+  } else {
+    defer iterCh.Close()
 
-  for rec := range iterCh.Records() {
-    fmt.Printf("%+v\n", rec)
+    for rec := range iterCh.Records() {
+      fmt.Printf("%+v\n", rec)
+    }
   }
 }
 ```
@@ -216,14 +217,15 @@ func main() {
     Reversed: true,
   }
 
-  iterCh, err := sm.CustomIterCh(false, time.Time{}, time.Now())
+  iterCh, err := sm.CustomIterCh(params)
   if err != nil {
     fmt.Println(err)
-  }
-  defer iterCh.Close()
+  } else {
+    defer iterCh.Close()
 
-  for rec := range iterCh.Records() {
-    fmt.Printf("%+v\n", rec)
+    for rec := range iterCh.Records() {
+      fmt.Printf("%+v\n", rec)
+    }
   }
 }
 ```
@@ -287,11 +289,12 @@ func main() {
   // BatchInsert the example records:
   sm.BatchInsert(records)
 
-  if !sm.BoundedIterFunc(false, time.Time{}, time.Now(), func(rec sortedmap.Record) bool {
+  if err := sm.BoundedIterFunc(false, time.Time{}, time.Now(), func(rec sortedmap.Record) bool {
     fmt.Printf("%+v\n", rec)
     return true
-  }) {
-    fmt.Println("No values fall within the specified bounds.")
+  })
+  err != nil {
+    fmt.Println(err)
   }
 }
 ```
@@ -357,12 +360,13 @@ func main() {
 
   // Copy the map + slice headers.
   m := sm.Map()
-  if keys, ok := sm.BoundedKeys(time.Time{}, time.Now()); ok {
+  keys, err := sm.BoundedKeys(time.Time{}, time.Now())
+  if err != nil {
+    fmt.Println(err)
+  } else {
     for _, k := range keys {
       fmt.Printf("%+v\n", m[k])
     }
-  } else {
-    fmt.Println("No values fall within the specified bounds.")
   }
 }
 ```

diff --git a/iter_test.go b/iter_test.go
@@ -249,13 +249,8 @@ func TestCustomIterCh(t *testing.T) {
 	}
 
 	func() {
-		ch, err := sm.CustomIterCh(params)
-		if err != nil {
-			t.Fatal(err)
-		}
-		defer ch.Close()
-
-		if err := verifyRecords(ch.Records(), params.Reversed); err != nil {
+		_, err := sm.CustomIterCh(params)
+		if err == nil {
 			t.Fatal(err)
 		}
 	}()
@@ -269,13 +264,8 @@ func TestCustomIterCh(t *testing.T) {
 	}
 
 	func() {
-		ch, err := sm.CustomIterCh(params)
-		if err != nil {
-			t.Fatal(err)
-		}
-		defer ch.Close()
-
-		if err := verifyRecords(ch.Records(), params.Reversed); err != nil {
+		_, err := sm.CustomIterCh(params)
+		if err == nil {
 			t.Fatal(err)
 		}
 	}()
@@ -395,7 +385,7 @@ func TestBoundedIterFunc(t *testing.T) {
 	}
 }
 
-func TestTestBoundedIterFuncWithNoBoundsReturned(t *testing.T) {
+func TestBoundedIterFuncWithNoBoundsReturned(t *testing.T) {
 	sm, _, err := newSortedMapFromRandRecords(1000)
 	if err != nil {
 		t.Fatal(err)
@@ -406,8 +396,9 @@ func TestTestBoundedIterFuncWithNoBoundsReturned(t *testing.T) {
 			t.Fatalf("TestBoundedIterFunc failed: %v", nilValErr)
 		}
 		return false
-	}); err == nil {
-		t.Fatal("Values fall between or are equal to the given bounds when it should not have returned bounds.")
+	})
+	err != nil {
+		t.Fatal(err)
 	}
 }
 

diff --git a/keys_test.go b/keys_test.go
@@ -49,7 +49,7 @@ func TestBoundedKeysWithNoBoundsReturned(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if val, err := sm.BoundedKeys(time.Now().Add(-1*time.Microsecond), time.Now()); err == nil {
-		t.Fatalf("Values fall between or are equal to the given bounds when it should not have returned bounds: %+v", sm.idx[val[0]])
+	if _, err := sm.BoundedKeys(time.Now().Add(-1*time.Microsecond), time.Now()); err != nil {
+		t.Fatal(err)
 	}
 }