From 624a8746cea9a5a27f9a21eebdd6be29de2cc69c Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Thu, 15 Nov 2018 13:06:12 +0100
Subject: [PATCH] ENH: code reduction in MeshObject (#1071)

- use forwarding templates for the factory method

- avoid double use of dynamic_cast.
  Don't need implicit use in isA<>, can use result directly

STYLE: updated iteration over HashTable of mesh objects
---
 src/OpenFOAM/meshes/MeshObject/MeshObject.C | 224 ++++----------------
 src/OpenFOAM/meshes/MeshObject/MeshObject.H |  52 +----
 2 files changed, 49 insertions(+), 227 deletions(-)

diff --git a/src/OpenFOAM/meshes/MeshObject/MeshObject.C b/src/OpenFOAM/meshes/MeshObject/MeshObject.C
index 6b21cf5b8f..0517be46ce 100644
--- a/src/OpenFOAM/meshes/MeshObject/MeshObject.C
+++ b/src/OpenFOAM/meshes/MeshObject/MeshObject.C
@@ -40,181 +40,36 @@ Foam::MeshObject<Mesh, MeshObjectType, Type>::MeshObject(const Mesh& mesh)
 // * * * * * * * * * * * * * * * * Selectors * * * * * * * * * * * * * * * * //
 
 template<class Mesh, template<class> class MeshObjectType, class Type>
-const Type& Foam::MeshObject<Mesh, MeshObjectType, Type>::New
-(
-    const Mesh& mesh
-)
-{
-    const Type* ptr = mesh.thisDb().objectRegistry::template cfindObject<Type>
-    (
-        Type::typeName
-    );
-
-    if (ptr)
-    {
-        return *ptr;
-    }
-    else
-    {
-        if (meshObject::debug)
-        {
-            Pout<< "MeshObject::New(const " << Mesh::typeName
-                << "&) : constructing " << Type::typeName
-                << " for region " << mesh.name() << endl;
-        }
-
-        Type* objectPtr = new Type(mesh);
-
-        regIOobject::store(static_cast<MeshObjectType<Mesh>*>(objectPtr));
-
-        return *objectPtr;
-    }
-}
-
-
-template<class Mesh, template<class> class MeshObjectType, class Type>
-template<class Data1>
+template<class... Args>
 const Type& Foam::MeshObject<Mesh, MeshObjectType, Type>::New
 (
     const Mesh& mesh,
-    const Data1& d
+    Args&&... args
 )
 {
-    const Type* ptr = mesh.thisDb().objectRegistry::template cfindObject<Type>
-    (
-        Type::typeName
-    );
+    const Type* ptr =
+        mesh.thisDb().objectRegistry::template cfindObject<Type>
+        (
+            Type::typeName
+        );
 
     if (ptr)
     {
         return *ptr;
     }
-    else
-    {
-        if (meshObject::debug)
-        {
-           Pout<< "MeshObject::New(const " << Mesh::typeName
-                << "&, const Data1&) : constructing " << Type::typeName
-                << " for region " << mesh.name() << endl;
-        }
-
-        Type* objectPtr = new Type(mesh, d);
-
-        regIOobject::store(static_cast<MeshObjectType<Mesh>*>(objectPtr));
-
-        return *objectPtr;
-    }
-}
-
-
-template<class Mesh, template<class> class MeshObjectType, class Type>
-template<class Data1, class Data2>
-const Type& Foam::MeshObject<Mesh, MeshObjectType, Type>::New
-(
-    const Mesh& mesh,
-    const Data1& d1,
-    const Data2& d2
-)
-{
-    const Type* ptr = mesh.thisDb().objectRegistry::template cfindObject<Type>
-    (
-        Type::typeName
-    );
-
-    if (ptr)
-    {
-        return *ptr;
-    }
-    else
-    {
-        if (meshObject::debug)
-        {
-           Pout<< "MeshObject::New(const " << Mesh::typeName
-                << "&, const Data[1-2]&) : constructing " << Type::typeName
-                << " for region " << mesh.name() << endl;
-        }
-
-        Type* objectPtr = new Type(mesh, d1, d2);
-
-        // Make sure to register the top level regIOobject for if Type itself
-        // is a regIOobject
-        regIOobject::store(static_cast<MeshObjectType<Mesh>*>(objectPtr));
 
-        return *objectPtr;
-    }
-}
-
-
-template<class Mesh, template<class> class MeshObjectType, class Type>
-template<class Data1, class Data2, class Data3>
-const Type& Foam::MeshObject<Mesh, MeshObjectType, Type>::New
-(
-    const Mesh& mesh,
-    const Data1& d1,
-    const Data2& d2,
-    const Data3& d3
-)
-{
-    const Type* ptr = mesh.thisDb().objectRegistry::template cfindObject<Type>
-    (
-        Type::typeName
-    );
-
-    if (ptr)
+    if (meshObject::debug)
     {
-        return *ptr;
+        Pout<< "MeshObject::New(const " << Mesh::typeName
+            << "&, ...) : constructing " << Type::typeName
+            << " for region " << mesh.name() << endl;
     }
-    else
-    {
-        if (meshObject::debug)
-        {
-           Pout<< "MeshObject::New(const " << Mesh::typeName
-                << "&, const Data[1-3]&) : constructing " << Type::typeName
-                << " for region " << mesh.name() << endl;
-        }
-        Type* objectPtr = new Type(mesh, d1, d2, d3);
-
-        regIOobject::store(static_cast<MeshObjectType<Mesh>*>(objectPtr));
-
-        return *objectPtr;
-    }
-}
-
-
-template<class Mesh, template<class> class MeshObjectType, class Type>
-template<class Data1, class Data2, class Data3, class Data4>
-const Type& Foam::MeshObject<Mesh, MeshObjectType, Type>::New
-(
-    const Mesh& mesh,
-    const Data1& d1,
-    const Data2& d2,
-    const Data3& d3,
-    const Data4& d4
-)
-{
-    const Type* ptr = mesh.thisDb().objectRegistry::template cfindObject<Type>
-    (
-        Type::typeName
-    );
 
-    if (ptr)
-    {
-        return *ptr;
-    }
-    else
-    {
-        if (meshObject::debug)
-        {
-            Pout<< "MeshObject::New(const " << Mesh::typeName
-                << "&, const Data[1-4]&) : constructing " << Type::typeName
-                << " for region " << mesh.name() << endl;
-        }
-        Type* objectPtr = new Type(mesh, d1, d2, d3, d4);
+    Type* objectPtr = new Type(mesh, std::forward<Args>(args)...);
 
-        regIOobject::store(static_cast<MeshObjectType<Mesh>*>(objectPtr));
+    regIOobject::store(static_cast<MeshObjectType<Mesh>*>(objectPtr));
 
-        return *objectPtr;
-    }
+    return *objectPtr;
 }
 
 
@@ -223,10 +78,11 @@ const Type& Foam::MeshObject<Mesh, MeshObjectType, Type>::New
 template<class Mesh, template<class> class MeshObjectType, class Type>
 bool Foam::MeshObject<Mesh, MeshObjectType, Type>::Delete(const Mesh& mesh)
 {
-    const Type* ptr = mesh.thisDb().objectRegistry::template findObject<Type>
-    (
-        Type::typeName
-    );
+    Type* ptr =
+        mesh.thisDb().objectRegistry::template getObjectPtr<Type>
+        (
+            Type::typeName
+        );
 
     if (ptr)
     {
@@ -236,12 +92,10 @@ bool Foam::MeshObject<Mesh, MeshObjectType, Type>::Delete(const Mesh& mesh)
                 << Type::typeName << endl;
         }
 
-        return mesh.thisDb().checkOut(const_cast<Type&>(*ptr));
-    }
-    else
-    {
-        return false;
+        return mesh.thisDb().checkOut(*ptr);
     }
+
+    return false;
 }
 
 
@@ -267,20 +121,19 @@ void Foam::meshObject::movePoints(objectRegistry& obr)
             << " meshObjects for region " << obr.name() << endl;
     }
 
-    forAllIter
-    (
-        typename HashTable<GeometricMeshObject<Mesh>*>,
-        meshObjects,
-        iter
-    )
+    forAllIters(meshObjects, iter)
     {
-        if (isA<MoveableMeshObject<Mesh>>(*iter()))
+        // Same as (isA<MoveableMeshObject<Mesh>>(*iter()))
+
+        auto* objectPtr = dynamic_cast<MoveableMeshObject<Mesh>*>(iter());
+
+        if (objectPtr)
         {
             if (meshObject::debug)
             {
                 Pout<< "    Moving " << iter()->name() << endl;
             }
-            dynamic_cast<MoveableMeshObject<Mesh>*>(iter())->movePoints();
+            objectPtr->movePoints();
         }
         else
         {
@@ -309,20 +162,19 @@ void Foam::meshObject::updateMesh(objectRegistry& obr, const mapPolyMesh& mpm)
             << " meshObjects for region " << obr.name() << endl;
     }
 
-    forAllIter
-    (
-        typename HashTable<GeometricMeshObject<Mesh>*>,
-        meshObjects,
-        iter
-    )
+    forAllIters(meshObjects, iter)
     {
-        if (isA<UpdateableMeshObject<Mesh>>(*iter()))
+        // Same as (isA<UpdateableMeshObject<Mesh>>(*iter()))
+
+        auto* objectPtr = dynamic_cast<UpdateableMeshObject<Mesh>*>(iter());
+
+        if (objectPtr)
         {
             if (meshObject::debug)
             {
                 Pout<< "    Updating " << iter()->name() << endl;
             }
-            dynamic_cast<UpdateableMeshObject<Mesh>*>(iter())->updateMesh(mpm);
+            objectPtr->updateMesh(mpm);
         }
         else
         {
@@ -351,7 +203,7 @@ void Foam::meshObject::clear(objectRegistry& obr)
             << " meshObjects for region " << obr.name() << endl;
     }
 
-    forAllIter(typename HashTable<MeshObjectType<Mesh>*>, meshObjects, iter)
+    forAllIters(meshObjects, iter)
     {
         if (meshObject::debug)
         {
@@ -382,7 +234,7 @@ void Foam::meshObject::clearUpto(objectRegistry& obr)
             << " meshObjects for region " << obr.name() << endl;
     }
 
-    forAllIter(typename HashTable<FromType<Mesh>*>, meshObjects, iter)
+    forAllIters(meshObjects, iter)
     {
         if (!isA<ToType<Mesh>>(*iter()))
         {
diff --git a/src/OpenFOAM/meshes/MeshObject/MeshObject.H b/src/OpenFOAM/meshes/MeshObject/MeshObject.H
index 84e66ae864..d3fed0b25c 100644
--- a/src/OpenFOAM/meshes/MeshObject/MeshObject.H
+++ b/src/OpenFOAM/meshes/MeshObject/MeshObject.H
@@ -3,7 +3,7 @@
   \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
    \\    /   O peration     |
     \\  /    A nd           | Copyright (C) 2011-2016 OpenFOAM Foundation
-     \\/     M anipulation  |
+     \\/     M anipulation  | Copyright (C) 2018 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -100,47 +100,17 @@ public:
 
         explicit MeshObject(const Mesh& mesh);
 
-        static const Type& New(const Mesh& mesh);
-
-        template<class Data1>
-        static const Type& New
-        (
-            const Mesh& mesh,
-            const Data1& d
-        );
-
-        template<class Data1, class Data2>
-        static const Type& New
-        (
-            const Mesh& mesh,
-            const Data1&,
-            const Data2&
-        );
-
-        template<class Data1, class Data2, class Data3>
-        static const Type& New
-        (
-            const Mesh& mesh,
-            const Data1&,
-            const Data2&,
-            const Data3&
-        );
-
-        template<class Data1, class Data2, class Data3, class Data4>
-        static const Type& New
-        (
-            const Mesh& mesh,
-            const Data1&,
-            const Data2&,
-            const Data3&,
-            const Data4&
-        );
-
-
-    // Destructor
-
-        virtual ~MeshObject();
+    // Selectors
 
+        //- Get existing or create a new MeshObject
+        template<class... Args>
+        static const Type& New(const Mesh& mesh, Args&&... args);
+
+
+    //- Destructor
+    virtual ~MeshObject();
+
+        //- Static destructor
         static bool Delete(const Mesh& mesh);
 
 
-- 
GitLab