Fix up edge cases in binsearch unit tests

Remove the start = NULL test cases, since it will no longer be
supported. Always return NA if key is NA or zero-length. Fix compiler
warning in binsearch.c ('static' is unnecessary as the struct is
already local to the file).

See joshuaulrich#100.

inst/unitTests/runit.binsearch.R

@@ -10,7 +10,6 @@ test.integer_predicate_no_yes_stops <- function() {
   ikey <- ivec[ans]
   checkIdentical(ans, xts:::binsearch(ikey, ivec, TRUE))
   checkIdentical(ans, xts:::binsearch(ikey, ivec, FALSE))
-  #checkIdentical(ans, xts:::binsearch(ikey, ivec, NULL))
 }
 
 # small steps between vector elements (test that we actually stop)
@@ -20,7 +19,6 @@ test.double_with_small_delta_stops <- function() {
   dkey <- dvec[ans]
   checkIdentical(ans, xts:::binsearch(dkey, dvec, TRUE))
   checkIdentical(ans, xts:::binsearch(dkey, dvec, FALSE))
-  #checkIdentical(ans, xts:::binsearch(dkey, dvec, NULL))
 }
 
 test.find_first_zero_even_length <- function() {
@@ -35,8 +33,6 @@ test.find_last_zero_even_length <- function() {
   dvec <- ivec * 1.0
   checkIdentical(5L, xts:::binsearch(0L,  ivec, FALSE))
   checkIdentical(5L, xts:::binsearch(0.0, dvec, FALSE))
-  #checkIdentical(5L, xts:::binsearch(0L,  ivec, NULL))
-  #checkIdentical(5L, xts:::binsearch(0.0, dvec, NULL))
 }
 
 test.find_first_zero_odd_length <- function() {
@@ -51,78 +47,65 @@ test.find_last_zero_odd_length <- function() {
   dvec <- ivec * 1.0
   checkIdentical(5L, xts:::binsearch(0L,  ivec, FALSE))
   checkIdentical(5L, xts:::binsearch(0.0, dvec, FALSE))
-  #checkIdentical(5L, xts:::binsearch(0L,  ivec, NULL))
-  #checkIdentical(5L, xts:::binsearch(0.0, dvec, NULL))
 }
 
 # key is outside of vector
 test.key_less_than_min <- function() {
-  # FIXME: should binsearch() return a value < length(vec)?
   ivec <- 1:6
   checkIdentical(1L, xts:::binsearch(-9L, ivec, TRUE))
-#  checkIdentical(0L, xts:::binsearch(-9L, ivec, FALSE))
-  #checkIdentical(na, xts:::binsearch(-9L, ivec, NULL))
+  checkIdentical(na, xts:::binsearch(-9L, ivec, FALSE))
   dvec <- ivec * 1.0
   checkIdentical(1L, xts:::binsearch(-9,  dvec, TRUE))
-#  checkIdentical(0L, xts:::binsearch(-9,  dvec, FALSE))
-  #checkIdentical(na, xts:::binsearch(-9,  dvec, NULL))
+  checkIdentical(na, xts:::binsearch(-9,  dvec, FALSE))
 }
 
 test.key_greater_than_max <- function() {
-  # FIXME: should binsearch() return a value > length(vec)?
   ivec <- 1:6
-#  checkIdentical(7L, xts:::binsearch( 9L, ivec, TRUE))
+  checkIdentical(na, xts:::binsearch( 9L, ivec, TRUE))
   checkIdentical(6L, xts:::binsearch( 9L, ivec, FALSE))
-  #checkIdentical(na, xts:::binsearch( 9L, ivec, NULL))
   dvec <- ivec * 1.0
-#  checkIdentical(7L, xts:::binsearch( 9,  dvec, TRUE))
+  checkIdentical(na, xts:::binsearch( 9,  dvec, TRUE))
   checkIdentical(6L, xts:::binsearch( 9,  dvec, FALSE))
-  #checkIdentical(na, xts:::binsearch( 9,  dvec, NULL))
 }
 
 # key is NA
 test.key_is_NA <- function() {
-  # FIXME: should binsearch() return non-NA if key is NA?
-  # NA_integer_ will be first and NA_real_ will be last in sorted vec
   ivec <- 1:6
   ikey <- NA_integer_
-#  checkIdentical(na, xts:::binsearch(ikey, ivec, TRUE))
+  checkIdentical(na, xts:::binsearch(ikey, ivec, TRUE))
   checkIdentical(na, xts:::binsearch(ikey, ivec, FALSE))
-  #checkIdentical(na, xts:::binsearch(ikey, ivec, NULL))
+
   dvec <- ivec * 1.0
   dkey <- NA_real_
   checkIdentical(na, xts:::binsearch(dkey, dvec, TRUE))
-#  checkIdentical(na, xts:::binsearch(dkey, dvec, FALSE))
-  #checkIdentical(na, xts:::binsearch(dkey, dvec, NULL))
+  checkIdentical(na, xts:::binsearch(dkey, dvec, FALSE))
 }
 
 # key is zero-length
 test.key_is_zero_length <- function() {
-  # TODO: determine what should happen
+  # have empty key return NA
   ivec <- 1:6
   ikey <- integer()
   checkIdentical(na, xts:::binsearch(ikey, ivec, TRUE))
-#  checkIdentical(na, xts:::binsearch(ikey, ivec, FALSE))
-  #checkIdentical(na, xts:::binsearch(ikey, ivec, NULL))
+  checkIdentical(na, xts:::binsearch(ikey, ivec, FALSE))
+
   dvec <- ivec * 1.0
   dkey <- double()
-#  checkIdentical(na, xts:::binsearch(dkey, dvec, TRUE))
+  checkIdentical(na, xts:::binsearch(dkey, dvec, TRUE))
   checkIdentical(na, xts:::binsearch(dkey, dvec, FALSE))
-  #checkIdentical(na, xts:::binsearch(dkey, dvec, NULL))
 }
 
 # vec is zero-length
 test.vec_is_zero_length <- function() {
-  # TODO: determine what should happen
+  # have empty vector return NA
   ivec <- integer()
   ikey <- 0L
   checkIdentical(na, xts:::binsearch(ikey, ivec, TRUE))
   checkIdentical(na, xts:::binsearch(ikey, ivec, FALSE))
-  #checkIdentical(na, xts:::binsearch(ikey, ivec, NULL))
+
   dvec <- double()
   dkey <- 0.0
   checkIdentical(na, xts:::binsearch(dkey, dvec, TRUE))
   checkIdentical(na, xts:::binsearch(dkey, dvec, FALSE))
-  #checkIdentical(na, xts:::binsearch(dkey, dvec, NULL))
 }
 

src/binsearch.c

@@ -6,7 +6,7 @@
  * contributions by Joshua Ulrich
  */
 
-static struct keyvec {
+struct keyvec {
   double *dvec;
   double dkey;
   int *ivec;
@@ -54,7 +54,7 @@ SEXP binsearch(SEXP key, SEXP vec, SEXP start)
     error("start must be specified as true or false");
   }
 
-  if (length(vec) < 1) {
+  if (length(vec) < 1 || length(key) < 1) {
     return ScalarInteger(NA_INTEGER);
   }
 
@@ -67,11 +67,17 @@ SEXP binsearch(SEXP key, SEXP vec, SEXP start)
       data.dkey = REAL(key)[0];
       data.dvec = REAL(vec);
       cmp_func = (use_start) ? cmp_dbl_lower : cmp_dbl_upper;
+      if (!R_finite(data.dkey)) {
+        return ScalarInteger(NA_INTEGER);
+      }
       break;
     case INTSXP:
       data.ikey = INTEGER(key)[0];
       data.ivec = INTEGER(vec);
       cmp_func = (use_start) ? cmp_int_lower : cmp_int_upper;
+      if (NA_INTEGER == data.ikey) {
+        return ScalarInteger(NA_INTEGER);
+      }
       break;
     default:
       error("unsupported type");