Refactor QPDF_Array::getItem and rename to at

5 files changed, 74 insertions, 88 deletions

diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc
index 9fd8684b..e08c675d 100644
--- a/libqpdf/QPDFObjectHandle.cc
+++ b/libqpdf/QPDFObjectHandle.cc
@@ -801,90 +801,88 @@ QPDFObjectHandle::getArrayNItems()
 QPDFObjectHandle
 QPDFObjectHandle::getArrayItem(int n)
 {
-    auto array = asArray();
-    if (array && n < array->size() && n >= 0) {
-        return array->getItem(n);
-    } else {
-        if (array) {
+    if (auto array = asArray()) {
+        if (auto result = array->at(n); result.obj != nullptr) {
+            return result;
+        } else {
             objectWarning("returning null for out of bounds array access");
             QTC::TC("qpdf", "QPDFObjectHandle array bounds");
-        } else {
-            typeWarning("array", "returning null");
-            QTC::TC("qpdf", "QPDFObjectHandle array null for non-array");
         }
-        static auto constexpr msg =
-            " -> null returned from invalid array access"sv;
-        return QPDF_Null::create(obj, msg, "");
+    } else {
+        typeWarning("array", "returning null");
+        QTC::TC("qpdf", "QPDFObjectHandle array null for non-array");
     }
+    static auto constexpr msg = " -> null returned from invalid array access"sv;
+    return QPDF_Null::create(obj, msg, "");
 }
 
 bool
 QPDFObjectHandle::isRectangle()
 {
-    auto array = asArray();
-    if (array == nullptr || array->size() != 4) {
-        return false;
-    }
-    for (int i = 0; i < 4; ++i) {
-        if (!array->getItem(i).isNumber()) {
-            return false;
+    if (auto array = asArray()) {
+        for (int i = 0; i < 4; ++i) {
+            if (auto item = array->at(i); !(item.obj && item.isNumber())) {
+                return false;
+            }
         }
+        return array->size() == 4;
     }
-    return true;
+    return false;
 }
 
 bool
 QPDFObjectHandle::isMatrix()
 {
-    auto array = asArray();
-    if (array == nullptr || array->size() != 6) {
-        return false;
-    }
-    for (int i = 0; i < 6; ++i) {
-        if (!array->getItem(i).isNumber()) {
-            return false;
+    if (auto array = asArray()) {
+        for (int i = 0; i < 6; ++i) {
+            if (auto item = array->at(i); !(item.obj && item.isNumber())) {
+                return false;
+            }
         }
+        return array->size() == 6;
     }
-    return true;
+    return false;
 }
 
 QPDFObjectHandle::Rectangle
 QPDFObjectHandle::getArrayAsRectangle()
 {
-    Rectangle result;
-    if (isRectangle()) {
-        auto array = asArray();
-        // Rectangle coordinates are always supposed to be llx, lly,
-        // urx, ury, but files have been found in the wild where
-        // llx > urx or lly > ury.
-        double i0 = array->getItem(0).getNumericValue();
-        double i1 = array->getItem(1).getNumericValue();
-        double i2 = array->getItem(2).getNumericValue();
-        double i3 = array->getItem(3).getNumericValue();
-        result = Rectangle(
-            std::min(i0, i2),
-            std::min(i1, i3),
-            std::max(i0, i2),
-            std::max(i1, i3));
+    if (auto array = asArray()) {
+        if (array->size() != 4) {
+            return {};
+        }
+        double items[4];
+        for (int i = 0; i < 4; ++i) {
+            if (!array->at(i).getValueAsNumber(items[i])) {
+                return {};
+            }
+        }
+        return Rectangle(
+            std::min(items[0], items[2]),
+            std::min(items[1], items[3]),
+            std::max(items[0], items[2]),
+            std::max(items[1], items[3]));
     }
-    return result;
+    return {};
 }
 
 QPDFObjectHandle::Matrix
 QPDFObjectHandle::getArrayAsMatrix()
 {
-    Matrix result;
-    if (isMatrix()) {
-        auto array = asArray();
-        result = Matrix(
-            array->getItem(0).getNumericValue(),
-            array->getItem(1).getNumericValue(),
-            array->getItem(2).getNumericValue(),
-            array->getItem(3).getNumericValue(),
-            array->getItem(4).getNumericValue(),
-            array->getItem(5).getNumericValue());
+    if (auto array = asArray()) {
+        if (array->size() != 6) {
+            return {};
+        }
+        double items[6];
+        for (int i = 0; i < 6; ++i) {
+            if (!array->at(i).getValueAsNumber(items[i])) {
+                return {};
+            }
+        }
+        return Matrix(
+            items[0], items[1], items[2], items[3], items[4], items[5]);
     }
-    return result;
+    return {};
 }
 
 std::vector<QPDFObjectHandle>
@@ -991,8 +989,8 @@ QPDFObjectHandle
 QPDFObjectHandle::eraseItemAndGetOld(int at)
 {
     auto array = asArray();
-    auto result = (array && at < array->size() && at >= 0) ? array->getItem(at)
-                                                           : newNull();
+    auto result =
+        (array && at < array->size() && at >= 0) ? array->at(at) : newNull();
     eraseItem(at);
     return result;
 }
@@ -1515,7 +1513,7 @@ QPDFObjectHandle::arrayOrStreamToStreamArray(
     if (auto array = asArray()) {
         int n_items = array->size();
         for (int i = 0; i < n_items; ++i) {
-            QPDFObjectHandle item = array->getItem(i);
+            QPDFObjectHandle item = array->at(i);
             if (item.isStream()) {
                 result.push_back(item);
             } else {
@@ -2215,7 +2213,7 @@ QPDFObjectHandle::makeDirect(
         auto array = asArray();
         int n = array->size();
         for (int i = 0; i < n; ++i) {
-            items.push_back(array->getItem(i));
+            items.push_back(array->at(i));
             items.back().makeDirect(visited, stop_at_streams);
         }
         this->obj = QPDF_Array::create(items);
diff --git a/libqpdf/QPDF_Array.cc b/libqpdf/QPDF_Array.cc
index 9368ca51..e9d216a5 100644
--- a/libqpdf/QPDF_Array.cc
+++ b/libqpdf/QPDF_Array.cc
@@ -105,7 +105,7 @@ QPDF_Array::unparse()
         std::string result = "[ ";
         int size = sp_elements.size();
         for (int i = 0; i < size; ++i) {
-            result += sp_elements.at(i).unparse();
+            result += at(i).unparse();
             result += " ";
         }
         result += "]";
@@ -114,7 +114,7 @@ QPDF_Array::unparse()
         std::string result = "[ ";
         auto size = elements.size();
         for (int i = 0; i < int(size); ++i) {
-            result += getItem(i).unparse();
+            result += at(i).unparse();
             result += " ";
         }
         result += "]";
@@ -129,35 +129,29 @@ QPDF_Array::getJSON(int json_version)
         JSON j = JSON::makeArray();
         int size = sp_elements.size();
         for (int i = 0; i < size; ++i) {
-            j.addArrayElement(sp_elements.at(i).getJSON(json_version));
+            j.addArrayElement(at(i).getJSON(json_version));
         }
         return j;
     } else {
         JSON j = JSON::makeArray();
         size_t size = elements.size();
         for (int i = 0; i < int(size); ++i) {
-            j.addArrayElement(getItem(i).getJSON(json_version));
+            j.addArrayElement(at(i).getJSON(json_version));
         }
         return j;
     }
 }
 
 QPDFObjectHandle
-QPDF_Array::getItem(int n) const
+QPDF_Array::at(int n) const noexcept
 {
-    if (sparse) {
-        if ((n < 0) || (n >= QIntC::to_int(sp_elements.size()))) {
-            throw std::logic_error(
-                "INTERNAL ERROR: bounds error accessing QPDF_Array element");
-        }
-        return sp_elements.at(n);
+    if (n < 0 || n >= size()) {
+        return {};
+    } else if (sparse) {
+        auto const& iter = sp_elements.elements.find(n);
+        return iter == sp_elements.elements.end() ? null_oh : (*iter).second;
     } else {
-        if ((n < 0) || (n >= QIntC::to_int(elements.size()))) {
-            throw std::logic_error(
-                "INTERNAL ERROR: bounds error accessing QPDF_Array element");
-        }
-        auto const& obj = elements.at(size_t(n));
-        return obj ? obj : null_oh;
+        return elements[size_t(n)];
     }
 }
 
@@ -167,7 +161,7 @@ QPDF_Array::getAsVector(std::vector<QPDFObjectHandle>& v) const
     if (sparse) {
         int size = sp_elements.size();
         for (int i = 0; i < size; ++i) {
-            v.push_back(sp_elements.at(i));
+            v.push_back(at(i));
         }
     } else {
         v = std::vector<QPDFObjectHandle>(elements.cbegin(), elements.cend());
diff --git a/libqpdf/SparseOHArray.cc b/libqpdf/SparseOHArray.cc
index 3f6376a6..c82bf145 100644
--- a/libqpdf/SparseOHArray.cc
+++ b/libqpdf/SparseOHArray.cc
@@ -2,6 +2,8 @@
 
 #include <stdexcept>
 
+static const QPDFObjectHandle null_oh = QPDFObjectHandle::newNull();
+
 void
 SparseOHArray::append(QPDFObjectHandle oh)
 {
@@ -23,16 +25,8 @@ SparseOHArray::append(std::shared_ptr<QPDFObject>&& obj)
 QPDFObjectHandle
 SparseOHArray::at(int idx) const
 {
-    if (idx < 0 || idx >= this->n_elements) {
-        throw std::logic_error(
-            "INTERNAL ERROR: bounds error accessing SparseOHArray element");
-    }
-    auto const& iter = this->elements.find(idx);
-    if (iter == this->elements.end()) {
-        return QPDFObjectHandle::newNull();
-    } else {
-        return (*iter).second;
-    }
+    auto const& iter = elements.find(idx);
+    return iter == elements.end() ? null_oh : (*iter).second;
 }
 
 void
diff --git a/libqpdf/qpdf/QPDF_Array.hh b/libqpdf/qpdf/QPDF_Array.hh
index 558704c7..4947d09c 100644
--- a/libqpdf/qpdf/QPDF_Array.hh
+++ b/libqpdf/qpdf/QPDF_Array.hh
@@ -27,7 +27,7 @@ class QPDF_Array: public QPDFValue
     {
         return sparse ? sp_elements.size() : int(elements.size());
     }
-    QPDFObjectHandle getItem(int n) const;
+    QPDFObjectHandle at(int n) const noexcept;
     void getAsVector(std::vector<QPDFObjectHandle>&) const;
 
     void setItem(int, QPDFObjectHandle const&);
diff --git a/libqpdf/qpdf/SparseOHArray.hh b/libqpdf/qpdf/SparseOHArray.hh
index 468e3a39..80de95e5 100644
--- a/libqpdf/qpdf/SparseOHArray.hh
+++ b/libqpdf/qpdf/SparseOHArray.hh
@@ -12,7 +12,7 @@ class SparseOHArray
   public:
     SparseOHArray() = default;
     int
-    size() const
+    size() const noexcept
     {
         return n_elements;
     }