Repository: NREL/OpenStudio (language: python)

Issue:
`SurfacePropertyExposedFoundationPerimeter` has surfaceName method instead of surface
Issue overview
--------------
Not sure if this is a bug report or a feature request...

The only way to get the surface from a `SurfacePropertyExposedFoundationPerimeter` object is from the `surfaceName` (string) method, which isn't very helpful. Instead, it should be a `surface` method that returns a Surface object. 

`SurfacePropertyIncidentSolarMultiplier` is a similar object that does it right -- it correctly has a `subSurface` method.



Discussion / hints:
Certainly seems like there should be a `surface` method - not sure why I originally implemented as `surfaceName`.

Fix patch:
diff --git a/src/energyplus/ForwardTranslator/ForwardTranslateSurfacePropertyExposedFoundationPerimeter.cpp b/src/energyplus/ForwardTranslator/ForwardTranslateSurfacePropertyExposedFoundationPerimeter.cpp
index 1bd45fb4175..196ee4d55ee 100644
--- a/src/energyplus/ForwardTranslator/ForwardTranslateSurfacePropertyExposedFoundationPerimeter.cpp
+++ b/src/energyplus/ForwardTranslator/ForwardTranslateSurfacePropertyExposedFoundationPerimeter.cpp
@@ -32,6 +32,7 @@
 #include "../../model/Model.hpp"
 #include "../../model/SurfacePropertyExposedFoundationPerimeter.hpp"
 #include "../../model/SurfacePropertyExposedFoundationPerimeter_Impl.hpp"
+#include "../../model/Surface.hpp"
 
 #include <utilities/idd/SurfaceProperty_ExposedFoundationPerimeter_FieldEnums.hxx>
 #include "../../utilities/idd/IddEnums.hpp"
@@ -51,7 +52,7 @@ namespace energyplus {
 
     m_idfObjects.push_back(idfObject);
 
-    idfObject.setString(SurfaceProperty_ExposedFoundationPerimeterFields::SurfaceName, modelObject.surfaceName());
+    idfObject.setString(SurfaceProperty_ExposedFoundationPerimeterFields::SurfaceName, modelObject.surface().nameString());
 
     idfObject.setString(SurfaceProperty_ExposedFoundationPerimeterFields::ExposedPerimeterCalculationMethod,
                         modelObject.exposedPerimeterCalculationMethod());
diff --git a/src/model/SurfacePropertyExposedFoundationPerimeter.cpp b/src/model/SurfacePropertyExposedFoundationPerimeter.cpp
index e44a33b1e09..f3be24e8841 100644
--- a/src/model/SurfacePropertyExposedFoundationPerimeter.cpp
+++ b/src/model/SurfacePropertyExposedFoundationPerimeter.cpp
@@ -71,11 +71,16 @@ namespace model {
       return SurfacePropertyExposedFoundationPerimeter::iddObjectType();
     }
 
-    std::string SurfacePropertyExposedFoundationPerimeter_Impl::surfaceName() const {
-      boost::optional<Surface> surface =
-        getObject<ModelObject>().getModelObjectTarget<Surface>(OS_SurfaceProperty_ExposedFoundationPerimeterFields::SurfaceName);
-      OS_ASSERT(surface);
-      return surface.get().name().get();
+    Surface SurfacePropertyExposedFoundationPerimeter_Impl::surface() const {
+      boost::optional<Surface> value = optionalSurface();
+      if (!value) {
+        LOG_AND_THROW(briefDescription() << " does not have an Surface attached.");
+      }
+      return value.get();
+    }
+
+    boost::optional<Surface> SurfacePropertyExposedFoundationPerimeter_Impl::optionalSurface() const {
+      return getObject<ModelObject>().getModelObjectTarget<Surface>(OS_SurfaceProperty_ExposedFoundationPerimeterFields::SurfaceName);
     }
 
     std::string SurfacePropertyExposedFoundationPerimeter_Impl::exposedPerimeterCalculationMethod() const {
@@ -170,7 +175,13 @@ namespace model {
   }
 
   std::string SurfacePropertyExposedFoundationPerimeter::surfaceName() const {
-    return getImpl<detail::SurfacePropertyExposedFoundationPerimeter_Impl>()->surfaceName();
+    LOG(Warn, "As of 3.7.0, surfaceName is deprecated. Use surface.nameString instead. It will be "
+              "removed within three releases.");
+    return getImpl<detail::SurfacePropertyExposedFoundationPerimeter_Impl>()->surface().nameString();
+  }
+
+  Surface SurfacePropertyExposedFoundationPerimeter::surface() const {
+    return getImpl<detail::SurfacePropertyExposedFoundationPerimeter_Impl>()->surface();
   }
 
   std::string SurfacePropertyExposedFoundationPerimeter::exposedPerimeterCalculationMethod() const {
diff --git a/src/model/SurfacePropertyExposedFoundationPerimeter.hpp b/src/model/SurfacePropertyExposedFoundationPerimeter.hpp
index 6bb97c38f0c..a6f1295a7d8 100644
--- a/src/model/SurfacePropertyExposedFoundationPerimeter.hpp
+++ b/src/model/SurfacePropertyExposedFoundationPerimeter.hpp
@@ -33,6 +33,7 @@
 #include <vector>
 #include "ModelAPI.hpp"
 #include "ModelObject.hpp"
+#include "../utilities/core/Deprecated.hpp"
 
 namespace openstudio {
 namespace model {
@@ -68,7 +69,9 @@ namespace model {
     /** @name Getters */
     //@{
 
-    std::string surfaceName() const;
+    OS_DEPRECATED std::string surfaceName() const;
+
+    Surface surface() const;
 
     std::string exposedPerimeterCalculationMethod() const;
 
diff --git a/src/model/SurfacePropertyExposedFoundationPerimeter_Impl.hpp b/src/model/SurfacePropertyExposedFoundationPerimeter_Impl.hpp
index 342c3794a14..d08be5f69d4 100644
--- a/src/model/SurfacePropertyExposedFoundationPerimeter_Impl.hpp
+++ b/src/model/SurfacePropertyExposedFoundationPerimeter_Impl.hpp
@@ -37,6 +37,8 @@
 namespace openstudio {
 namespace model {
 
+  class Surface;
+
   namespace detail {
 
     /** SurfacePropertyExposedFoundationPerimeter_Impl is a ModelObject_Impl that is the implementation class for SurfacePropertyExposedFoundationPerimeter.*/
@@ -67,7 +69,7 @@ namespace model {
       /** @name Getters */
       //@{
 
-      std::string surfaceName() const;
+      Surface surface() const;
 
       std::string exposedPerimeterCalculationMethod() const;
 
@@ -101,6 +103,8 @@ namespace model {
      protected:
      private:
       REGISTER_LOGGER("openstudio.model.SurfacePropertyExposedFoundationPerimeter");
+
+      boost::optional<Surface> optionalSurface() const;
     };
 
   }  // namespace detail


Test patch:
diff --git a/src/model/test/SurfacePropertyExposedFoundationPerimeter_GTest.cpp b/src/model/test/SurfacePropertyExposedFoundationPerimeter_GTest.cpp
index 87a101a159f..5f833e3fd3a 100644
--- a/src/model/test/SurfacePropertyExposedFoundationPerimeter_GTest.cpp
+++ b/src/model/test/SurfacePropertyExposedFoundationPerimeter_GTest.cpp
@@ -195,9 +195,11 @@ TEST_F(ModelFixture, SurfacePropertyExposedFoundationPerimeter_SurfaceName) {
     surface.createSurfacePropertyExposedFoundationPerimeter("TotalExposedPerimeter", 100);
   auto prop = optprop.get();
   EXPECT_EQ("Surface 1", prop.surfaceName());
+  EXPECT_EQ(surface.handle(), prop.surface().handle());
   Surface surface2(points, model);
   boost::optional<SurfacePropertyExposedFoundationPerimeter> optprop2 =
     surface2.createSurfacePropertyExposedFoundationPerimeter("TotalExposedPerimeter", 100);
   auto prop2 = optprop2.get();
   EXPECT_EQ("Surface 2", prop2.surfaceName());
+  EXPECT_EQ(surface2.handle(), prop2.surface().handle());
 }