From f2199af0eba57a001d196d96bac0ad66d1d1d842 Mon Sep 17 00:00:00 2001
From: Patrick Shriwise <pshriwise@gmail.com>
Date: Fri, 19 Jul 2019 09:46:47 -0500
Subject: [PATCH] Cleaning up error checking in bounding_box. Updating expected
 exceptions in the capi tests.

---
 src/geometry.cpp              | 19 ++++++++++++++-----
 tests/unit_tests/test_capi.py |  6 +++---
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/src/geometry.cpp b/src/geometry.cpp
index 91e2fc245d..ae48a0acb9 100644
--- a/src/geometry.cpp
+++ b/src/geometry.cpp
@@ -484,18 +484,27 @@ openmc_bounding_box(const char* geom_type, const int32_t id, double* llc, double
   std::string gtype(geom_type);
   to_lower(gtype);
 
+  // negative ids only for surfaces
+  if (id < 0 && gtype != "surface") {
+    std::stringstream msg;
+    msg << "Negative ID " << id << " passed to bounding box for "
+        << "non-surface geom type '" << geom_type << "'" << std::endl;
+    set_errmsg(msg);
+    return OPENMC_E_INVALID_ID;
+  }
+  // id should never be zero
+  if (id == 0) {
+    set_errmsg("Invalid ID 0 for surface bounding box.");
+    return OPENMC_E_INVALID_ID;
+  }
+
   if (gtype == "universe") {
-    // negative ids only apply to surfaces
-    if (id <= 0) { return OPENMC_E_INVALID_ID; }
     const auto& u = model::universes[model::universe_map.at(id)];
     bbox = u->bounding_box();
   } else if (gtype == "cell") {
-    // negative ids only apply to surfaces
-    if (id <= 0) { return OPENMC_E_INVALID_ID; }
     const auto& c = model::cells[model::cell_map.at(id)];
     bbox = c->bounding_box();
   } else if (gtype == "surface") {
-    if (id == 0) { return OPENMC_E_INVALID_ID; }
     const auto& s = model::surfaces[model::surface_map.at(abs(id))];
     bbox = s->bounding_box(id > 0);
   } else {
diff --git a/tests/unit_tests/test_capi.py b/tests/unit_tests/test_capi.py
index b02e43815f..aa140a121b 100644
--- a/tests/unit_tests/test_capi.py
+++ b/tests/unit_tests/test_capi.py
@@ -508,13 +508,13 @@ def test_bounding_box(capi_init):
     assert tuple(urc) == expected_urc
 
     # make sure that proper assertions are raised
-    with pytest.raises(openmc.exceptions.GeometryError):
+    with pytest.raises(openmc.exceptions.InvalidIDError):
         openmc.capi.bounding_box("Cell", -1)
 
-    with pytest.raises(openmc.exceptions.GeometryError):
+    with pytest.raises(openmc.exceptions.InvalidIDError):
         openmc.capi.bounding_box("Surface", 0)
 
-    with pytest.raises(openmc.exceptions.GeometryError):
+    with pytest.raises(openmc.exceptions.InvalidTypeError):
         openmc.capi.bounding_box("Region", 1)