bug fix to be more type-safe (fix issue #2)

ropensci · Jul 4, 2017 · a6000e0 · a6000e0
1 parent 371f16d
commit a6000e0
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 7 deletions.
diff --git a/R/commodity_lookup.R b/R/commodity_lookup.R
@@ -111,7 +111,7 @@ commodity_lookup <- function(values, lookuptable, return_code = FALSE,
   # If ans is a list, check the legnth of each element. If any have length
   # zero, create a warning message that will be printed to console if input
   # param "verbose" is TRUE. If there are no elements with length zero,
-  # convert ans to a char vector is return_char is TRUE.
+  # convert ans to a char vector if return_char is TRUE.
   if (is.list(ans)) {
     if (verbose) {
       check_len <- vapply(ans, length, integer(1), USE.NAMES = FALSE)
@@ -131,7 +131,16 @@ commodity_lookup <- function(values, lookuptable, return_code = FALSE,
     }
   }
 
-  # Return ans. If obj msg exists, print warning message.
+  # If ans is a char vector, convert to a list if return_char is FALSE, otherwise
+  # unname the char vector.
+  if (mode(ans) == "character") {
+    if (return_char) {
+      ans <- unname(ans)
+    } else{
+      ans <- lapply(ans, function(x) x)
+    }
+  }
+
   if (exists("msg")) {
     warning(msg, call. = FALSE)
   }

diff --git a/tests/testthat/test-commodity_lookup.R b/tests/testthat/test-commodity_lookup.R
@@ -6,35 +6,45 @@ test_that("lookup return values are correct, and fail when expected", {
   skip_on_travis()
 
   df <- ct_commodities_table(type = "HS")
-  ex_1 <- commodity_lookup(value = "halibut",
+  ex_1 <- commodity_lookup(values = "halibut",
                            lookuptable = df,
                            return_code = FALSE,
                            return_char = TRUE,
                            verbose = TRUE)
-  ex_2 <- commodity_lookup(value = 1602,
+  ex_2 <- commodity_lookup(values = 1602,
                            lookuptable = df,
                            return_code = TRUE,
                            return_char = FALSE,
                            verbose = TRUE)
+  ex_3 <- commodity_lookup(values = df$commodity[c(10, 20, 40)],
+                           lookuptable = df,
+                           return_code = TRUE,
+                           return_char = TRUE,
+                           verbose = TRUE)
 
   # Correct return data type.
   expect_is(ex_1, "character")
   expect_is(ex_2, "list")
+  expect_is(ex_3, "character")
 
   # Number of return values.
   expect_equal(length(ex_1), 2)
   expect_equal(length(ex_2), 1)
+  expect_equal(length(ex_3), 3)
+
+  # Make sure example 3 is not a named vector.
+  expect_named(ex_3, NULL)
 
   # Correct return values when input for "value" not found in lookup table.
-  expect_warning(ex_3 <- commodity_lookup(value = "not_a_trade_item",
+  expect_warning(ex_4 <- commodity_lookup(values = "not_a_trade_item",
                                           lookuptable = df,
                                           return_char = TRUE))
-  expect_equal(length(ex_3), 0)
+  expect_equal(length(ex_4), 0)
 
   # Throw error with invalid input for param "value".
   expect_error(commodity_lookup(value = list(), lookuptable = df))
 
   # Throw error with invalid input for param "lookuptable".
-  expect_error(commodity_lookup(value = "halibut",
+  expect_error(commodity_lookup(values = "halibut",
                                 lookuptable = "lookuptable"))
 })