From 8433ddee0e3753b0b52f7b49a8bfbc6b3e26d799 Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Thu, 24 Jan 2019 17:00:12 +0100
Subject: [PATCH] ENH: simplify coding in functionObjectList

- use for-range when iterating
- dictionary access/search methods
- autoPtr for memory management
---
 .../functionObjectList/functionObjectList.C   | 195 +++++++++---------
 .../functionObjectList/functionObjectList.H   |  23 ++-
 2 files changed, 108 insertions(+), 110 deletions(-)

diff --git a/src/OpenFOAM/db/functionObjects/functionObjectList/functionObjectList.C b/src/OpenFOAM/db/functionObjects/functionObjectList/functionObjectList.C
index 8da391cc8d..30bfd63210 100644
--- a/src/OpenFOAM/db/functionObjects/functionObjectList/functionObjectList.C
+++ b/src/OpenFOAM/db/functionObjects/functionObjectList/functionObjectList.C
@@ -3,7 +3,7 @@
   \\      /  F ield         | OpenFOAM: The Open Source CFD Toolbox
    \\    /   O peration     |
     \\  /    A nd           | Copyright (C) 2011-2017 OpenFOAM Foundation
-     \\/     M anipulation  | Copyright (C) 2015-2018 OpenCFD Ltd.
+     \\/     M anipulation  | Copyright (C) 2015-2019 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -67,31 +67,30 @@ void Foam::functionObjectList::createStateDict() const
 }
 
 
-Foam::functionObject* Foam::functionObjectList::remove
+Foam::autoPtr<Foam::functionObject> Foam::functionObjectList::remove
 (
     const word& key,
     label& oldIndex
 )
 {
-    functionObject* ptr = nullptr;
+    autoPtr<functionObject> oldptr;
 
-    // Find index of existing functionObject
-    HashTable<label>::iterator fnd = indices_.find(key);
+    auto iter = indices_.find(key);  // Index of existing functionObject
 
-    if (fnd != indices_.end())
+    if (iter.found())
     {
-        oldIndex = fnd();
+        oldIndex = *iter;
 
-        // Retrieve the pointer and remove it from the old list
-        ptr = this->set(oldIndex, nullptr).ptr();
-        indices_.erase(fnd);
+        // Remove pointer from the old list
+        oldptr = this->set(oldIndex, nullptr);
+        indices_.erase(iter);
     }
     else
     {
         oldIndex = -1;
     }
 
-    return ptr;
+    return oldptr;
 }
 
 
@@ -179,7 +178,7 @@ bool Foam::functionObjectList::readFunctionObject
     wordReList args;
 
     List<Tuple2<word, string>> namedArgs;
-    bool namedArg = false;
+    bool hasNamedArg = false;
     word argName;
 
     word::size_type start = 0;
@@ -207,7 +206,7 @@ bool Foam::functionObjectList::readFunctionObject
         {
             if (argLevel == 1)
             {
-                if (namedArg)
+                if (hasNamedArg)
                 {
                     namedArgs.append
                     (
@@ -217,7 +216,7 @@ bool Foam::functionObjectList::readFunctionObject
                             funcNameArgs.substr(start, i - start)
                         )
                     );
-                    namedArg = false;
+                    hasNamedArg = false;
                 }
                 else
                 {
@@ -252,14 +251,14 @@ bool Foam::functionObjectList::readFunctionObject
             );
 
             start = i+1;
-            namedArg = true;
+            hasNamedArg = true;
         }
 
         ++i;
     }
 
     // Search for the functionObject dictionary
-    fileName path = findDict(funcName);
+    fileName path = functionObjectList::findDict(funcName);
 
     if (path == fileName::null)
     {
@@ -269,19 +268,13 @@ bool Foam::functionObjectList::readFunctionObject
     }
 
     // Read the functionObject dictionary
-    //IFstream fileStream(path);
     autoPtr<ISstream> fileStreamPtr(fileHandler().NewIFstream(path));
     ISstream& fileStream = fileStreamPtr();
 
     dictionary funcsDict(fileStream);
-    dictionary* funcDictPtr = &funcsDict;
+    dictionary* funcDictPtr = funcsDict.findDict(funcName);
+    dictionary& funcDict = (funcDictPtr ? *funcDictPtr : funcsDict);
 
-    if (funcsDict.found(funcName) && funcsDict.isDict(funcName))
-    {
-        funcDictPtr = &funcsDict.subDict(funcName);
-    }
-
-    dictionary& funcDict = *funcDictPtr;
 
     // Insert the 'field' and/or 'fields' entry corresponding to the optional
     // arguments or read the 'field' or 'fields' entry and add the required
@@ -307,12 +300,13 @@ bool Foam::functionObjectList::readFunctionObject
     }
 
     // Insert named arguments
-    forAll(namedArgs, i)
+    for (const Tuple2<word, string>& namedArg : namedArgs)
     {
         IStringStream entryStream
         (
-            namedArgs[i].first() + ' ' + namedArgs[i].second() + ';'
+            namedArg.first() + ' ' + namedArg.second() + ';'
         );
+
         funcDict.set(entry::New(entryStream).ptr());
     }
 
@@ -508,12 +502,16 @@ void Foam::functionObjectList::clear()
 
 Foam::label Foam::functionObjectList::findObjectID(const word& name) const
 {
-    forAll(*this, objectI)
+    label id = 0;
+
+    for (const functionObject& funcObj : functions())
     {
-        if (operator[](objectI).name() == name)
+        if (funcObj.name() == name)
         {
-            return objectI;
+            return id;
         }
+
+        ++id;
     }
 
     return -1;
@@ -528,7 +526,7 @@ void Foam::functionObjectList::on()
 
 void Foam::functionObjectList::off()
 {
-    // For safety, also force a read() when execution is turned back on
+    // For safety, also force a read() when execution is resumed
     updated_ = execution_ = false;
 }
 
@@ -556,19 +554,19 @@ bool Foam::functionObjectList::execute()
             read();
         }
 
-        forAll(*this, objectI)
+        for (functionObject& funcObj : functions())
         {
-            const word& objName = operator[](objectI).name();
+            const word& objName = funcObj.name();
             {
                 addProfiling(fo, "functionObject::" + objName + "::execute");
 
-                ok = operator[](objectI).execute() && ok;
+                ok = funcObj.execute() && ok;
             }
 
             {
                 addProfiling(fo, "functionObject::" + objName + "::write");
 
-                ok = operator[](objectI).write() && ok;
+                ok = funcObj.write() && ok;
             }
         }
     }
@@ -600,10 +598,8 @@ bool Foam::functionObjectList::execute(const label subIndex)
 
     if (ok)
     {
-        forAll(*this, obji)
+        for (functionObject& funcObj : functions())
         {
-            functionObject& funcObj = operator[](obji);
-
             ok = funcObj.execute(subIndex) && ok;
         }
     }
@@ -622,10 +618,8 @@ bool Foam::functionObjectList::execute
 
     if (ok && functionNames.size())
     {
-        forAll(*this, obji)
+        for (functionObject& funcObj : functions())
         {
-            functionObject& funcObj = operator[](obji);
-
             if (stringOps::match(functionNames, funcObj.name()))
             {
                 ok = funcObj.execute(subIndex) && ok;
@@ -648,13 +642,13 @@ bool Foam::functionObjectList::end()
             read();
         }
 
-        forAll(*this, objectI)
+        for (functionObject& funcObj : functions())
         {
-            const word& objName = operator[](objectI).name();
+            const word& objName = funcObj.name();
 
             addProfiling(fo, "functionObject::" + objName + "::end");
 
-            ok = operator[](objectI).end() && ok;
+            ok = funcObj.end() && ok;
         }
     }
 
@@ -673,13 +667,13 @@ bool Foam::functionObjectList::adjustTimeStep()
             read();
         }
 
-        forAll(*this, objectI)
+        for (functionObject& funcObj : functions())
         {
-            const word& objName = operator[](objectI).name();
+            const word& objName = funcObj.name();
 
             addProfiling(fo, "functionObject::" + objName + "::adjustTimeStep");
 
-            ok = operator[](objectI).adjustTimeStep() && ok;
+            ok = funcObj.adjustTimeStep() && ok;
         }
     }
 
@@ -694,7 +688,6 @@ bool Foam::functionObjectList::read()
         createStateDict();
     }
 
-    bool ok = true;
     updated_ = execution_;
 
     // Avoid reading/initializing if execution is off
@@ -707,24 +700,32 @@ bool Foam::functionObjectList::read()
     const entry* entryPtr =
         parentDict_.findEntry("functions", keyType::LITERAL);
 
-    if (entryPtr)
+    bool ok = true;
+
+    if (!entryPtr)
     {
-        PtrList<functionObject> newPtrs;
-        List<SHA1Digest> newDigs;
+        // No functions
+        PtrList<functionObject>::clear();
+        digests_.clear();
+        indices_.clear();
+    }
+    else if (!entryPtr->isDict())
+    {
+        // Bad entry type
+        ok = false;
+        FatalIOErrorInFunction(parentDict_)
+            << "'functions' entry is not a dictionary"
+            << exit(FatalIOError);
+    }
+    else
+    {
+        const dictionary& functionsDict = entryPtr->dict();
+
+        PtrList<functionObject> newPtrs(functionsDict.size());
+        List<SHA1Digest> newDigs(functionsDict.size());
         HashTable<label> newIndices;
 
-        label nFunc = 0;
-
-        addProfiling(fo,"functionObjects::read");
-
-        if (!entryPtr->isDict())
-        {
-            FatalIOErrorInFunction(parentDict_)
-                << "'functions' entry is not a dictionary"
-                << exit(FatalIOError);
-        }
-
-        const dictionary& functionsDict = entryPtr->dict();
+        addProfiling(fo, "functionObjects::read");
 
         const_cast<Time&>(time_).libs().open
         (
@@ -733,8 +734,7 @@ bool Foam::functionObjectList::read()
             functionObject::dictionaryConstructorTablePtr_
         );
 
-        newPtrs.setSize(functionsDict.size());
-        newDigs.setSize(functionsDict.size());
+        label nFunc = 0;
 
         for (const entry& dEntry : functionsDict)
         {
@@ -757,32 +757,29 @@ bool Foam::functionObjectList::read()
 
             newDigs[nFunc] = dict.digest();
 
-            label oldIndex;
-            functionObject* objPtr = remove(key, oldIndex);
+            label oldIndex = -1;
+            autoPtr<functionObject> objPtr = remove(key, oldIndex);
 
             if (objPtr)
             {
-                if (enabled)
+                // Re-read if dictionary content changed for
+                // existing functionObject
+                if (enabled && newDigs[nFunc] != digests_[oldIndex])
                 {
-                    // Dictionary changed for an existing functionObject
-                    if (newDigs[nFunc] != digests_[oldIndex])
-                    {
-                        addProfiling
-                        (
-                            fo2,
-                            "functionObject::" + objPtr->name() + "::read"
-                        );
+                    addProfiling
+                    (
+                        fo2,
+                        "functionObject::" + objPtr->name() + "::read"
+                    );
 
-                        enabled = objPtr->read(dict);
-                        ok = enabled && ok;
-                    }
+                    enabled = objPtr->read(dict);
+                    ok = enabled && ok;
                 }
 
                 if (!enabled)
                 {
-                    // Delete the disabled/invalid(read) functionObject
-                    delete objPtr;
-                    objPtr = nullptr;
+                    // Delete disabled or an invalid(read) functionObject
+                    objPtr.clear();
                     continue;
                 }
             }
@@ -823,7 +820,8 @@ bool Foam::functionObjectList::read()
                 {
                     // Bit of trickery to get the original message
                     err.write(Warning, false);
-                    InfoInFunction << nl
+                    InfoInFunction
+                        << nl
                         << "--> while loading function object '" << key << "'"
                         << nl << endl;
                 }
@@ -832,11 +830,10 @@ bool Foam::functionObjectList::read()
                 FatalError.throwExceptions(throwingError);
                 FatalIOError.throwExceptions(throwingIOerr);
 
-                // If one processor only has thrown an exception (so exited the
-                // constructor) invalidate the whole functionObject
+                // Required functionObject to be valid on all processors
                 if (returnReduce(foPtr.valid(), andOp<bool>()))
                 {
-                    objPtr = foPtr.ptr();
+                    objPtr.reset(foPtr.release());
                 }
                 else
                 {
@@ -844,30 +841,24 @@ bool Foam::functionObjectList::read()
                 }
             }
 
-            // Insert active functionObjects into the list
+            // Insert active functionObject into the list
             if (objPtr)
             {
                 newPtrs.set(nFunc, objPtr);
                 newIndices.insert(key, nFunc);
-                nFunc++;
+                ++nFunc;
             }
         }
 
-        newPtrs.setSize(nFunc);
-        newDigs.setSize(nFunc);
+        newPtrs.resize(nFunc);
+        newDigs.resize(nFunc);
 
-        // Updating the PtrList of functionObjects deletes any
+        // Updating PtrList of functionObjects deletes any
         // existing unused functionObjects
         PtrList<functionObject>::transfer(newPtrs);
         digests_.transfer(newDigs);
         indices_.transfer(newIndices);
     }
-    else
-    {
-        PtrList<functionObject>::clear();
-        digests_.clear();
-        indices_.clear();
-    }
 
     return ok;
 }
@@ -878,9 +869,9 @@ bool Foam::functionObjectList::filesModified() const
     bool ok = false;
     if (execution_)
     {
-        forAll(*this, objectI)
+        for (const functionObject& funcObj : functions())
         {
-            bool changed = operator[](objectI).filesModified();
+            bool changed = funcObj.filesModified();
             ok = ok || changed;
         }
     }
@@ -892,9 +883,9 @@ void Foam::functionObjectList::updateMesh(const mapPolyMesh& mpm)
 {
     if (execution_)
     {
-        forAll(*this, objectI)
+        for (functionObject& funcObj : functions())
         {
-            operator[](objectI).updateMesh(mpm);
+            funcObj.updateMesh(mpm);
         }
     }
 }
@@ -904,9 +895,9 @@ void Foam::functionObjectList::movePoints(const polyMesh& mesh)
 {
     if (execution_)
     {
-        forAll(*this, objectI)
+        for (functionObject& funcObj : functions())
         {
-            operator[](objectI).movePoints(mesh);
+            funcObj.movePoints(mesh);
         }
     }
 }
diff --git a/src/OpenFOAM/db/functionObjects/functionObjectList/functionObjectList.H b/src/OpenFOAM/db/functionObjects/functionObjectList/functionObjectList.H
index 5cb135612f..872bc02bbe 100644
--- a/src/OpenFOAM/db/functionObjects/functionObjectList/functionObjectList.H
+++ b/src/OpenFOAM/db/functionObjects/functionObjectList/functionObjectList.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  | Copyright (C) 2015-2018 OpenCFD Ltd.
+     \\/     M anipulation  | Copyright (C) 2015-2019 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -73,6 +73,7 @@ class functionObjectList
         //- Quick lookup of the index into functions/digests
         HashTable<label> indices_;
 
+        //- Reference to Time
         const Time& time_;
 
         //- The parent dictionary containing a "functions" entry
@@ -92,13 +93,19 @@ class functionObjectList
 
     // Private Member Functions
 
-        //- Create state dictionary
+        //- List of functions
+        const PtrList<functionObject>& functions() const { return *this; }
+
+        //- List of functions
+        PtrList<functionObject>& functions() { return *this; }
+
+        //- Create state dictionary - attached to Time.
         void createStateDict() const;
 
         //- Remove and return the function object pointer by name,
-        //- and returns the old index via the parameter.
-        //  Returns a nullptr (and index -1) if it didn't exist
-        functionObject* remove(const word& key, label& oldIndex);
+        //- and returns the old index (into digest) via the parameter.
+        //  Returns nullptr (and index -1) if it didn't exist
+        autoPtr<functionObject> remove(const word& key, label& oldIndex);
 
         //- Search the specified directory for functionObject
         //- configuration files, add to the given map and recurse
@@ -180,16 +187,16 @@ public:
         //- Reset/read state dictionary for current time
         void resetState();
 
-        //- Return the state dictionary
+        //- Write access to the state dictionary ("functionObjectProperties")
         IOdictionary& stateDict();
 
-        //- Return const access to the state dictionary
+        //- Const access to the state dictionary ("functionObjectProperties")
         const IOdictionary& stateDict() const;
 
         //- Clear the list of function objects
         void clear();
 
-        //- Find the ID of a given function object by name
+        //- Find the ID of a given function object by name, -1 if not found.
         label findObjectID(const word& name) const;
 
         //- Print a list of functionObject configuration files in the