Merge pull request #63 from elliotcmorris/work/57-language-consistency

C++/Python getTraitProperty consistency.

RELEASE_NOTES.md

@@ -1,6 +1,16 @@
 Release Notes
 =============
 
+v1.0.0-alpha.x
+--------------
+
+### Breaking Changes
+
+- C++ `getProperty` methods now return `std::optional<T>` rather
+  than throwing when invoked without a default parameter. This mirrors
+  the python behaviour of returning `None` in this case.
+  [#57](https://github.com/OpenAssetIO/OpenAssetIO-TraitGen/issues/57)
+
 v1.0.0-alpha.8
 --------------
 

python/openassetio_traitgen/templates/cpp/trait.hpp.in

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include <optional>
 #include <stdexcept>
 #include <utility>
 
@@ -109,19 +110,19 @@ public:
   }
 
   /**
-   * Gets the value of the {{ property.id }} property or throws if
-   * not found or is of an unexpected type.
+   * Gets the value of the {{ property.id }} property. Returns an empty
+   * optional if not found or is of an unexpected type.
    *
    * {{ property.description |  wordwrap(69, wrapstring="\n* ") | indent(4) }}
    */
-  [[nodiscard]] {{ VarType }} get{{ VarMethodName }}() const {
+  [[nodiscard]] std::optional<{{ VarType }}> get{{ VarMethodName }}() const {
     if (property::Value value; traitsData_->getTraitProperty(&value, kId, "{{ property.id }}")) {
       if ({{ VarType }}* maybeOut = std::get_if<{{ VarType }}>(&value)) {
         return *maybeOut;
       }
       throw std::runtime_error{"Invalid stored value type: should be '{{ VarType }}'."};
     }
-    throw std::runtime_error{"Property is not set."};
+    return std::optional<{{VarType}}>{};
   }
     {%-  endfor %}
   {%- endif %}

tests/generators/cpp/src/test.cpp

@@ -328,9 +328,16 @@ TEMPLATE_TEST_CASE("Property getters", "", openassetio_abi::Bool, openassetio_ab
     const Fixture fixture{AllPropertiesTrait{traitsData}};
 
     WHEN("property is queried without a default") {
-      const PropertyType value = fixture.getProperty();
-
-      THEN("value is as expected") { CHECK(value == Fixture::kExpectedValue); }
+      const std::optional<PropertyType> value = fixture.getProperty();
+
+      THEN("value is as expected") {
+        REQUIRE(value.has_value());
+        // Guard required because clang-tidy isn't clever enough to
+        // understand the require.
+        if (value.has_value()) {
+          CHECK(value.value() == Fixture::kExpectedValue);
+        }
+      }
     }
 
     WHEN("property is queried with a default") {
@@ -347,10 +354,7 @@ TEMPLATE_TEST_CASE("Property getters", "", openassetio_abi::Bool, openassetio_ab
     const Fixture fixture{AllPropertiesTrait{traitsData}};
 
     WHEN("property is queried without a default") {
-      THEN("exception is thrown") {
-        CHECK_THROWS_MATCHES(fixture.getProperty(), std::runtime_error,
-                             Catch::Matchers::Message("Property is not set."));
-      }
+      THEN("optional return is empty") { CHECK(!fixture.getProperty().has_value()); }
     }
 
     WHEN("property is queried with a default") {
@@ -369,10 +373,7 @@ TEMPLATE_TEST_CASE("Property getters", "", openassetio_abi::Bool, openassetio_ab
     const Fixture fixture{AllPropertiesTrait{traitsData}};
 
     WHEN("property is queried without a default") {
-      THEN("exception is thrown") {
-        CHECK_THROWS_MATCHES(fixture.getProperty(), std::runtime_error,
-                             Catch::Matchers::Message("Property is not set."));
-      }
+      THEN("optional return is empty") { CHECK(!fixture.getProperty().has_value()); }
     }
 
     WHEN("property is queried with a default") {

tests/generators/test_cpp.py

@@ -181,8 +181,8 @@ def test_has_prefixed_property_getters_with_default_with_expected_docstring(
             )
             == f"""
   /**
-   * Gets the value of the {property_name} property or throws if
-   * not found or is of an unexpected type.
+   * Gets the value of the {property_name} property. Returns an empty
+   * optional if not found or is of an unexpected type.
    *
    * A {property_type}-typed property.
    */

tests/generators/test_python.py

@@ -440,6 +440,22 @@ def test_when_property_is_set_then_returns_expected_value(
 
         assert getter() == property_.valid_value
 
+    def test_when_trait_not_set_then_returns_None(
+        self, all_properties_trait, an_empty_traitsData, property_
+    ):
+        a_trait = all_properties_trait(an_empty_traitsData)
+        getter = getattr(a_trait, f"get{property_.name[0].upper()}{property_.name[1:]}")
+
+        assert getter() is None
+
+    def test_when_trait_not_set_and_default_given_then_returns_default(
+        self, all_properties_trait, an_empty_traitsData, property_
+    ):
+        a_trait = all_properties_trait(an_empty_traitsData)
+        getter = getattr(a_trait, f"get{property_.name[0].upper()}{property_.name[1:]}")
+
+        assert getter(defaultValue=property_.valid_value) == property_.valid_value
+
     def test_when_property_not_set_then_returns_None(
         self, all_properties_trait, an_all_properties_traitsData, property_
     ):