From fcc7f702305a8d099f3c0b565be9e912491674f7 Mon Sep 17 00:00:00 2001 From: m-holger Date: Mon, 10 Nov 2025 10:31:09 +0000 Subject: [PATCH] Refactor: replace dictionary access via operator[] with `get` method for consistency and clarity. --- libqpdf/QPDFAnnotationObjectHelper.cc | 4 ++-- libqpdf/QPDFFileSpecObjectHelper.cc | 6 +++--- libqpdf/QPDFOutlineObjectHelper.cc | 4 ++-- libqpdf/QPDF_Dictionary.cc | 4 ++-- libtests/objects.cc | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++----------- libtests/qtest/objects.test | 8 ++++++-- 6 files changed, 85 insertions(+), 22 deletions(-) diff --git a/libqpdf/QPDFAnnotationObjectHelper.cc b/libqpdf/QPDFAnnotationObjectHelper.cc index 63e3628..9467ffd 100644 --- a/libqpdf/QPDFAnnotationObjectHelper.cc +++ b/libqpdf/QPDFAnnotationObjectHelper.cc @@ -33,14 +33,14 @@ QPDFAnnotationObjectHelper::getAppearanceDictionary() std::string QPDFAnnotationObjectHelper::getAppearanceState() { - Name AS = (*this)["/AS"]; + Name AS = get("/AS"); return AS ? AS.value() : ""; } int QPDFAnnotationObjectHelper::getFlags() { - Integer flags_obj = (*this)["/F"]; + Integer flags_obj = get("/F"); return flags_obj ? flags_obj : 0; } diff --git a/libqpdf/QPDFFileSpecObjectHelper.cc b/libqpdf/QPDFFileSpecObjectHelper.cc index 570b056..09c8df3 100644 --- a/libqpdf/QPDFFileSpecObjectHelper.cc +++ b/libqpdf/QPDFFileSpecObjectHelper.cc @@ -42,7 +42,7 @@ std::string QPDFFileSpecObjectHelper::getFilename() { for (auto const& i: name_keys) { - if (String k = oh()[i]) { + if (String k = get(i)) { return k.utf8_value(); } } @@ -54,7 +54,7 @@ QPDFFileSpecObjectHelper::getFilenames() { std::map result; for (auto const& i: name_keys) { - if (String k = oh()[i]) { + if (String k = get(i)) { result[i] = k.utf8_value(); } } @@ -64,7 +64,7 @@ QPDFFileSpecObjectHelper::getFilenames() QPDFObjectHandle QPDFFileSpecObjectHelper::getEmbeddedFileStream(std::string const& key) { - if (Dictionary EF = oh()["/EF"]) { + if (Dictionary EF = get("/EF")) { if (!key.empty() && EF.contains(key)) { if (auto result = EF[key]) { return result; diff --git a/libqpdf/QPDFOutlineObjectHelper.cc b/libqpdf/QPDFOutlineObjectHelper.cc index 3a163b6..f098804 100644 --- a/libqpdf/QPDFOutlineObjectHelper.cc +++ b/libqpdf/QPDFOutlineObjectHelper.cc @@ -53,9 +53,9 @@ QPDFOutlineObjectHelper::getKids() QPDFObjectHandle QPDFOutlineObjectHelper::getDest() { - auto dest = (*this)["/Dest"]; + auto dest = get("/Dest"); if (dest.null()) { - auto const& A = (*this)["/A"]; + auto const& A = get("/A"); if (Name(A["/S"]) == "/GoTo") { dest = A["/D"]; } diff --git a/libqpdf/QPDF_Dictionary.cc b/libqpdf/QPDF_Dictionary.cc index 830956f..b9c9d9d 100644 --- a/libqpdf/QPDF_Dictionary.cc +++ b/libqpdf/QPDF_Dictionary.cc @@ -191,7 +191,7 @@ QPDFObjectHandle::hasKey(std::string const& key) const QPDFObjectHandle QPDFObjectHandle::getKey(std::string const& key) const { - if (auto result = (*this)[key]) { + if (auto result = get(key)) { return result; } if (isDictionary()) { @@ -269,7 +269,7 @@ QPDFObjectHandle::removeKey(std::string const& key) QPDFObjectHandle QPDFObjectHandle::removeKeyAndGetOld(std::string const& key) { - auto result = (*this)[key]; + auto result = get(key); erase(key); return result ? result : newNull(); } diff --git a/libtests/objects.cc b/libtests/objects.cc index ee9c7c5..6f20aee 100644 --- a/libtests/objects.cc +++ b/libtests/objects.cc @@ -54,35 +54,30 @@ test_0(QPDF& pdf, char const* arg2) assert_compare_numbers(INT_MAX, t.getKey("/Q1").getIntValueAsInt()); try { assert_compare_numbers(0u, QPDFObjectHandle::newNull().getUIntValueAsUInt()); - std::cerr << "convert null to uint did not throw\n"; } catch (QPDFExc const&) { std::cerr << "caught expected type error\n"; } assert_compare_numbers(std::numeric_limits::max(), Integer(q1).value()); assert_compare_numbers(std::numeric_limits::min(), Integer(-q1).value()); try { - int8_t q1_8 = Integer(q1); - std::cerr << "q1_8: " << std::to_string(q1_8) << '\n'; + [[maybe_unused]] int8_t q1_8 = Integer(q1); } catch (std::overflow_error const&) { std::cerr << "caught expected int8_t overflow error\n"; } try { - int8_t q1_8 = Integer(-q1); - std::cerr << "q1_8: " << std::to_string(q1_8) << '\n'; + [[maybe_unused]] int8_t q1_8 = Integer(-q1); } catch (std::underflow_error const&) { std::cerr << "caught expected int8_t underflow error\n"; } assert_compare_numbers(std::numeric_limits::max(), Integer(q1).value()); assert_compare_numbers(0, Integer(-q1).value()); try { - uint8_t q1_u8 = Integer(q1); - std::cerr << "q1_u8: " << std::to_string(q1_u8) << '\n'; + [[maybe_unused]] uint8_t q1_u8 = Integer(q1); } catch (std::overflow_error const&) { std::cerr << "caught expected uint8_t overflow error\n"; } try { - uint8_t q1_u8 = Integer(-q1); - std::cerr << "q1_u8: " << std::to_string(q1_u8) << '\n'; + [[maybe_unused]] uint8_t q1_u8 = Integer(-q1); } catch (std::underflow_error const&) { std::cerr << "caught expected uint8_t underflow error\n"; } @@ -95,6 +90,70 @@ test_0(QPDF& pdf, char const* arg2) assert_compare_numbers(UINT_MAX, t.getKey("/Q3").getUIntValueAsUInt()); } + +static void +test_1(QPDF& pdf, char const* arg2) +{ + // Test new dictionary methods. + using namespace qpdf; + auto d = Dictionary({{"/A", {}}, {"/B", Null()}, {"/C", Dictionary::empty()}}); + + // contains + assert(!d.contains("/A")); + assert(!d.contains("/B")); + assert(d.contains("/C")); + + auto i = Integer(42); + assert(!i.contains("/A")); + + // at + assert(!d.at("/A")); + assert(d.at("/B")); + assert(d.at("/B").null()); + assert(d.at("/C")); + assert(!d.at("/C").null()); + d.at("/C") = Integer(42); + assert(d.at("/C") == 42); + assert(!d.at("/D")); + assert(d.at("/D").null()); + assert(QPDFObjectHandle(d).getDictAsMap().contains("/D")); + assert(QPDFObjectHandle(d).getDictAsMap().size() == 4); + + bool thrown = false; + try { + i.at("/A"); + } catch (std::runtime_error const&) { + thrown = true; + } + assert(thrown); + + // find + assert(!d.find("/A")); + assert(d.find("/B")); + assert(d.find("/B").null()); + assert(d.find("/C")); + assert(Integer(d.find("/C")) == 42); + d.find("/C") = Name("/DontPanic"); + assert(Name(d.find("/C")) == "/DontPanic"); + assert(!d.find("/E")); + assert(!QPDFObjectHandle(d).getDictAsMap().contains("/E")); + assert(QPDFObjectHandle(d).getDictAsMap().size() == 4); + + // replace + assert(!i.replace("/A", Name("/DontPanic"))); + Dictionary di = i.oh(); + thrown = false; + try { + di.replace("/A", Name("/DontPanic")); + } catch (std::runtime_error const&) { + thrown = true; + } + assert(thrown); + d.replace("/C", Integer(42)); + assert(Integer(d["/C"]) == 42); + assert(QPDFObjectHandle(d).getDictAsMap().size() == 4); +} + void runtest(int n, char const* filename1, char const* arg2) { @@ -102,7 +161,7 @@ runtest(int n, char const* filename1, char const* arg2) // the test suite to see how the test is invoked to find the file // that the test is supposed to operate on. - std::set ignore_filename = {}; + std::set ignore_filename = {1,}; QPDF pdf; std::shared_ptr file_buf; @@ -116,7 +175,7 @@ runtest(int n, char const* filename1, char const* arg2) } std::map test_functions = { - {0, test_0}, + {0, test_0}, {1, test_1}, }; auto fn = test_functions.find(n); diff --git a/libtests/qtest/objects.test b/libtests/qtest/objects.test index 7dec720..f6fde63 100644 --- a/libtests/qtest/objects.test +++ b/libtests/qtest/objects.test @@ -11,12 +11,16 @@ require TestDriver; my $td = new TestDriver('objects'); -my $n_tests = 1; - +my $n_tests = 2; $td->runtest("integer type checks", {$td->COMMAND => "objects 0 minimal.pdf"}, {$td->FILE => "test0.out", $td->EXIT_STATUS => 0}, $td->NORMALIZE_NEWLINES); +$td->runtest("dictionary checks", + {$td->COMMAND => "objects 1 -"}, + {$td->STRING => => "test 1 done\n", $td->EXIT_STATUS => 0}, + $td->NORMALIZE_NEWLINES); + $td->report($n_tests); -- libgit2 0.21.4