Reverse Polish notation calculator similar to dc on Unix/Linux systems using dynamic libraries

Question

This problem is using dynamic libraries so that additional calculator functions can be added by dropping a library into a specific directory.

What I'd like to get out of this code review is:

What do I still need to do to make this more C++ and less C?
Is the object oriented nature of the code good, or am I missing something in the object oriented design?
Is the debug and test part of the code something I should keep around or toss?
I've used the boost headers and libraries in some portions of the code to decrease the amount of code I need to write and make it more portable. I can't find anything to make the portion of the program that deals with dynamic/shared libraries more portable. Is there some library I can use to be able to port the code from Linux/Unix to Windows and Mac?

An example of what I'm looking for:

This morning looking at other questions I found out about nullptr. I should have used nullptr rather than NULL in the constructor in RpnDLData.cpp.

I started using C++ in 1989, ten years before C++98 was implemented. I never learned C++98, C++03 or C++11 until now.

To decrease the amount of code here in the question, I have excluded the objects that deal with I/O or the Operating System Interface (relies heavily on boost for portability (parsing command lines or environment variables)).

TstDbgCommon.h

#ifndef TSTDBGCOMMON_H_
#define TSTDBGCOMMON_H_

const unsigned int NODEBUGORTEST = 0;
const unsigned int NODEBUG = 0;
const unsigned int NOTEST = 0;
const unsigned int DEFAULTOBJECTTESTLEVEL = 1;
const unsigned int DEFAULTOBJECTDEBUGLEVEL = 0;
const unsigned int LEVEL1 = 1;
const unsigned int LEVEL2 = 2;
const unsigned int LEVEL3 = 3;
const unsigned int LEVEL4 = 4;
const unsigned int LEVEL5 = 5;
const unsigned int LEVEL6 = 6;
const unsigned int LEVEL7 = 7;
const unsigned int LEVEL8 = 8;
const unsigned int LEVEL9 = 9;
const unsigned int LEVEL10 = 10;
const unsigned int LEVEL11 = 11;
const unsigned int LEVEL12 = 12;
const unsigned int LEVEL13 = 13;
const unsigned int LEVEL14 = 14;
const unsigned int LEVEL15 = 15;
const unsigned int LEVEL16 = 16;
const unsigned int LEVEL17 = 17;
const unsigned int LEVEL18 = 18;
const unsigned int LEVEL19 = 19;
const unsigned int LEVEL20 = 20;

#endif /* TSTDBGCOMMON_H_ */

TestBase.h

#ifndef TESTBASE_H_
#define TESTBASE_H_

#include "TstDbgCommon.h"
using namespace std;
class TestBase
{
private:
    static unsigned int mA_Level;
    unsigned int mA_ObjectMinimumLevel;
    inline unsigned int mF_CheckLevelAgainstObjectLevel(unsigned int level)
    {
        return (mA_ObjectMinimumLevel < level);
    };
    inline unsigned int mF_CheckLevelAgainstProjectLevel(unsigned int level)
    {
        return (GetProjectTestLevel() > level);
    };
    inline unsigned int mF_CheckLevel(unsigned int level)
    {
        return (
            (mF_GetObjectLevel() < level) &&
            (level < GetProjectTestLevel())
        );
    };
protected:
    void mF_SetLevel(unsigned int Level)
    {
        this->mA_Level = Level;
    };
    inline unsigned int  mF_GetLevel()
    {
        return this->mA_Level;
    };
    inline void mF_SetObjectLevel(unsigned int Level)
    {
        this->mA_ObjectMinimumLevel = Level;
    };
    inline unsigned int  mF_GetObjectLevel()
    {
        return this->mA_ObjectMinimumLevel;
    };
    inline unsigned int mF_CheckTestLevel(unsigned int level)
    {
        return mF_CheckLevel(level);
    };
public:
    TestBase();
    virtual ~TestBase();
    inline void SetProjectTestLevel(unsigned int TestLevel)
    {
        mF_SetLevel(TestLevel);
    };
    inline unsigned int GetProjectTestLevel()
    {
        return mF_GetLevel();
    };
    inline void SetObjectTestLevel(unsigned int level)
    {
        mF_SetObjectLevel(level);
    };
    inline unsigned int GetObjectTestLevel()
    {
        return mF_GetObjectLevel();
    };
    inline unsigned int IsTesting()
    {
        return (
            (this->mA_Level) ?
                (
                    (this->mA_ObjectMinimumLevel) ?
                    (
                        this->mA_Level >= this->mA_ObjectMinimumLevel
                    )
                    : 0
                )
            : 0
        );
    };
    void ObjectLevelTesting(const char *OutputBuffer);
    virtual void ShowOnlyIfLevelGreaterThan(int forceLevel,
        const char* format, ...) = 0;
    virtual void TestReportThisObject() = 0;
};

#endif /* TESTBASE_H_ */

TestBase.cpp

#include "TestBase.h"
#include <iostream>
using namespace std;
unsigned int TestBase::mA_Level = NOTEST;
TestBase::TestBase() {
    mA_ObjectMinimumLevel = DEFAULTOBJECTTESTLEVEL;
}
TestBase::~TestBase() {
}

void TestBase::ObjectLevelTesting(const char *OutputBuffer)
{
    if (IsTesting())
    {
        cout << OutputBuffer;
    }
}

DebugBase.h

#ifndef DEBUGBASE_H_
#define DEBUGBASE_H_
#include "TstDbgCommon.h"
#include <functional>
using namespace std;
class DebugBase
{
private:
    static unsigned int mA_Level;
    unsigned int mA_ObjectMinimumLevel;
protected:
    inline void mF_SetProjectLevel(unsigned int Level)
    {
        this->mA_Level = Level;
    };
    inline unsigned int  mF_GetProjectLevel()
    {
        return this->mA_Level;
    };
    inline void  mF_SetObjectLevel(unsigned int level)
    {
        this->mA_ObjectMinimumLevel = level;
    };
    inline unsigned int  mF_GetObjectLevel()
    {
        return this->mA_ObjectMinimumLevel;
    };
    inline unsigned int mF_CheckLevelAgainstObjectLevel(unsigned int level)
    {
        return (mF_GetObjectLevel() < level);
    };
    inline unsigned int mF_CheckLevelAgainstProjectLevel(unsigned int level)
    {
        return (mF_GetProjectLevel() > level);
    };
    inline unsigned int mF_CheckLevel(unsigned int level)
    {
        return (
            (mF_GetObjectLevel() < level) &&
            (level < mF_GetProjectLevel())
        );
    };
public:
   DebugBase();
    virtual ~DebugBase();
    inline void SetProjectDebugLevel(unsigned int TestLevel)
    {
        mF_SetProjectLevel(TestLevel);
    };
    inline unsigned int GetProjectDebugLevel()
    {
        return mF_GetProjectLevel();
    };
    inline void SetObjectDebugLevel(unsigned int TestLevel)
    {
        mF_SetObjectLevel(TestLevel);
    };
    inline unsigned int GetObjectDebugLevel()
    {
        return mF_GetObjectLevel();
    };
    inline unsigned int IsDebugging()
    {
        return (
            (this->mA_Level) ?
            (
                (this->mA_ObjectMinimumLevel) ?
                (this->mA_Level >= this->mA_ObjectMinimumLevel)
                : 0)
            : 0
        );
    };
    void ObjectLevelDebugging(const char *OutputBuffer);
    virtual void ShowOnlyIfLevelGreaterThan(int forceLevel, const char* format, ...) = 0;
};
#endif /* DEBUGBASE_H_ */

DebugBase.cpp

#include "DebugBase.h"
#include <iostream>
using namespace std;
unsigned int DebugBase::mA_Level = 0;
DebugBase::DebugBase() {
    SetObjectDebugLevel(0);

}

DebugBase::~DebugBase() {
}

void DebugBase::ObjectLevelDebugging(const char *OutputBuffer)
{
    if (IsDebugging()) {
        cout << OutputBuffer;
    }
}

DbgTstHandling.h

#ifndef DBGTSTHANDLING_H_
#define DBGTSTHANDLING_H_
#include "TestBase.h"
#include "DebugBase.h"
#include <vector>
using namespace std;
class DebugAndTestHandling : protected TestBase, protected DebugBase
{
private:
    void mF_CommonDebugAndTestReporting(const char *Output);
    inline int mF_CheckLevelAgainstObjectLevels(unsigned int level)
    {
        return (
            ((TestBase::mF_GetObjectLevel() > level)) ||
            (DebugBase::mF_GetObjectLevel()> level)
        );
    };
    inline int mF_CheckLevelAgainstProjectDebug(unsigned int level)
    {
        return (GetProjectDebugLevel() > level);
    };
    inline int mF_CheckLevelAgainstProjectTest(unsigned int level)
    {
        return (GetProjectTestLevel() > level);
    };
    inline int mF_CheckLevelAgainstProjectLevels(unsigned int level)
    {
        return (
            (mF_CheckLevelAgainstProjectDebug(level)) ||
            (mF_CheckLevelAgainstProjectTest(level))
        );
    };
    inline int mF_CheckLevels(int level)
    {
        return (
            (TestBase::mF_CheckTestLevel(level)) ||
            (DebugBase::mF_CheckLevel(level))
        );
    };
protected:
    void ObjectLevelDebuggingOrTesting(const char *format, ...);
    void ShowOnlyIfLevelGreaterThan(int forceLevel, const char* format, ...);
    inline unsigned int IsTestingOrDebugging()
    {
        return ((IsTesting()) || (IsDebugging()));
    };
public:
    DebugAndTestHandling();
    virtual ~DebugAndTestHandling();
    inline void SetObjectMinimumDebugOrTestLevel(int level)
    {
        TestBase::SetObjectTestLevel(level);
        DebugBase::SetObjectDebugLevel(level);
    };
};

#endif /* DBGTSTHANDLING_H_ */

DbgTstHandling.cpp

#include "DbgTstHandling.h"
#include <iostream>
#include <cstdarg>
#include <ctype.h>
#include <string.h>
DebugAndTestHandling::DebugAndTestHandling()
{
    TestBase::SetObjectTestLevel(DEFAULTOBJECTTESTLEVEL);
    DebugBase::SetObjectDebugLevel(DEFAULTOBJECTDEBUGLEVEL);
}

DebugAndTestHandling::~DebugAndTestHandling()
{

}
void DebugAndTestHandling::mF_CommonDebugAndTestReporting(
        const char *OutputBuffer)
{
    if (IsDebugging())
    {
        ObjectLevelDebugging(OutputBuffer);
        return;
    }

    if (IsTesting())
    {
        ObjectLevelTesting(OutputBuffer);
        return;
    }
}
void DebugAndTestHandling::ObjectLevelDebuggingOrTesting(
        const char *format, ...)
{
    if (IsTestingOrDebugging())
    {
        char localBuffer[2028];
        va_list args;
        va_start(args, format);
        vsprintf(localBuffer, format, args);
        mF_CommonDebugAndTestReporting(localBuffer);
        va_end(args);
    }
}
void DebugAndTestHandling::ShowOnlyIfLevelGreaterThan(int forceLevel,
        const char *format, ...)
{
    if (mF_CheckLevels(forceLevel))
    {
        // Indent output by level
        for (int TabOutput = forceLevel; --TabOutput; )
    {
            cout << "\t";
        }
        char localBuffer[2028];
        va_list args;
        va_start(args, format);
        vsprintf(localBuffer, format, args);
        mF_CommonDebugAndTestReporting(localBuffer);
        va_end(args);
    }
}

RpnDLData.h

#ifndef RPNDLDATA_H_
#define RPNDLDATA_H_
#include <string>
using namespace std;
#include "plugins.h"
#include "DbgTstHandling.h"

using namespace std;
class RpnDLData : protected DebugAndTestHandling {
private:
    void *m_LibHandle;
    string *m_LibPath;
    OpTableEntry *m_Data;
    void m_OpenLibrary();
    void m_CloseLibrary();
    void m_FindRpnHubSymbol();
public:
    RpnDLData(string FullLibraryPath,
        int ObjectDebugTestLevel=DEFAULTOBJECTTESTLEVEL);
    virtual ~RpnDLData();
    inline const string *GetPath() { return m_LibPath; };
    inline int IsLibraryOpen() { return ((m_LibHandle) ? 1 : 0); };
    const OpTableEntry *GetOperationTableData();
    inline int IsRpnLibrary() { return (IsLibraryOpen() && m_Data); };
    void TestReportThisObject();
    void Test_ReportM_LibHandle();
    void Test_ReportM_LibPath();
    void Test_ReportM_Data();
    void Test_Reportm_IsOpen();
};

#endif /* RPNDLDATA_H_ */

RpnDLData.cpp

#include <iostream>
#include <dlfcn.h>
#include <string>
#include <typeinfo>
#include "RpnDLData.h"
using namespace std;
void RpnDLData::Test_Reportm_IsOpen()
{
    ObjectLevelDebuggingOrTesting("\tRpnDLData->m_IsOpen: %d\n",
        IsLibraryOpen());
}
void RpnDLData::Test_ReportM_LibPath()
{
    ObjectLevelDebuggingOrTesting("\tRpnDLData->m_LibPath %s\n",
        m_LibPath->c_str());
}
void RpnDLData::Test_ReportM_LibHandle()
{
    ObjectLevelDebuggingOrTesting("\tRpnDLData->m_LibHandle 0x%x\n", m_LibHandle);
}
void RpnDLData::Test_ReportM_Data()
{
    if (m_Data)
    {
        ObjectLevelDebuggingOrTesting("\tRpnDLData->m_Data = 0x%x\n", m_Data);
        ObjectLevelDebuggingOrTesting("\tRpnDLData->m_Data->FuncPtr = 0x%x\n",
            m_Data->FuncPtr);
        ObjectLevelDebuggingOrTesting("\tRpnDLData->m_Data->name = %s\n",
            m_Data->name);
    }
    else
    {
        ObjectLevelDebuggingOrTesting("\tRpnDLData->m_data = NULL\n");
    }
}
void RpnDLData::TestReportThisObject()
{
    ObjectLevelDebuggingOrTesting("RPN Dynamic Library Object Test Report\n");
    ObjectLevelDebuggingOrTesting("\tRpnDLData->TestLevel = %d\n",
        GetObjectTestLevel());
    ObjectLevelDebuggingOrTesting("\tRpnDLData->DebugLevel = %d\n",
        GetObjectDebugLevel());
    Test_ReportM_LibPath();
    Test_Reportm_IsOpen();
    Test_ReportM_Data();
    Test_ReportM_LibHandle();
}
RpnDLData::RpnDLData(string FullLibraryPath, int ObjectDebugTestLevel)
    {
    m_LibPath = NULL;
    m_LibHandle = NULL;
    m_Data = NULL;
    SetObjectMinimumDebugOrTestLevel(ObjectDebugTestLevel);
    m_LibPath = new string(FullLibraryPath);
    m_OpenLibrary();
    m_FindRpnHubSymbol();
    TestReportThisObject();
}
RpnDLData::~RpnDLData()
    {
    m_CloseLibrary();
    delete m_LibPath;
}
void RpnDLData::m_CloseLibrary()
{
    if (m_LibHandle)
    {
        dlclose(m_LibHandle);
        m_LibHandle = 0;
    }
}
void RpnDLData::m_OpenLibrary()
{
    if (!IsLibraryOpen())
    {
        if (!(m_LibHandle = dlopen(m_LibPath->c_str(), (RTLD_NOW | RTLD_LOCAL))))
        {
            ShowOnlyIfLevelGreaterThan(LEVEL3,
                "Can't open shared library %s\n", m_LibPath);
        }
    }
}
void RpnDLData::m_FindRpnHubSymbol()
{
    if (IsLibraryOpen())
    {
        if (m_Data)
        {
            return;
        }
        void *Found = NULL;
        dlerror();    // Clear any previous errors
        Found = dlsym(m_LibHandle, "rpnhub_plugin");
        if (Found)
        {
            OpTableEntry *TableEntry = static_cast<OpTableEntry *>(Found);
            if (!TableEntry)
            {
                m_CloseLibrary();
                ShowOnlyIfLevelGreaterThan(LEVEL3,
                    "Symbol does not convert %s\n", m_LibPath);
            }
            else
            {
                ShowOnlyIfLevelGreaterThan(LEVEL3,
                    "TableEntry = {%s,0x%x}\n", TableEntry->name,
                    TableEntry->FuncPtr);
            }
            m_Data = TableEntry;
        }
        else
        {
            ShowOnlyIfLevelGreaterThan(LEVEL3,
               "Can't find symbol rpnhub_plugin %s\n", m_LibPath);
        }
    }
}
const OpTableEntry *RpnDLData::GetOperationTableData()
{
    if (!IsRpnLibrary())
    {
        m_OpenLibrary();
        m_FindRpnHubSymbol();
    }
    return m_Data;
}

RpnOpsTab.h

#ifndef RPNOPSTAB_H_
#define RPNOPSTAB_H_
using namespace std;
#include <vector>
#include <stack>
#include <string>
#include <map>
#include "plugins.h"
#include "DbgTstHandling.h"
class RpnOperationsTable : protected DebugAndTestHandling {
private:
    string *m_SearchPath;
    map<string, const OpTableEntry *> m_Operations;
    vector<class RpnDLData *> m_OpenedLibraries;
    int m_FindAndAddPluginLibraries();
    int m_AddLibraryToTable(string Library);
    void m_CloseAllLibraries();

public:
    RpnOperationsTable(string *LibPath,
        int ObjectDebugTestLevel=DEFAULTOBJECTTESTLEVEL);
    virtual ~RpnOperationsTable();
    void ExecuteOperation(string InputToken, stack<double>& Operands);
    void TestReportThisObject();
};

#endif /* RPNOPSTAB_H_ */

RpnOpsTab.cpp

#include <stdexcept>
#include <cstdlib>
#include <error_code.hpp>
#include <range.hpp>
#include <filesystem.hpp>
#include "RpnDLData.h"
#include "RpnOpsTab.h"
using namespace std;
using namespace boost;
using namespace boost::system;
using namespace boost::filesystem;
using namespace boost::range;
RpnOperationsTable::RpnOperationsTable(string *PathSpec, int ObjectDebugTestLevel)
{
    SetObjectMinimumDebugOrTestLevel(ObjectDebugTestLevel);
    if (!PathSpec)
    {
        string Emsg = "In RpnOperationsTable Constructor: Drop in directory path not specified";
        throw runtime_error(Emsg);
    }
    m_SearchPath = new string(*PathSpec);
    ObjectLevelDebuggingOrTesting("Current Path = %s\n", m_SearchPath->c_str());

    if (!m_FindAndAddPluginLibraries())
    {
        string Emsg = "No plugin libraries found for rpn in the search directory ";
        Emsg.append(*m_SearchPath);
        throw runtime_error(Emsg);
    }
}
RpnOperationsTable::~RpnOperationsTable()
    {
    m_CloseAllLibraries();
    delete m_SearchPath;
}
void RpnOperationsTable::m_CloseAllLibraries()
{
    for (auto OpenSharedLib : m_OpenedLibraries)
    {
        RpnDLData *DLCloseData = OpenSharedLib;
        ShowOnlyIfLevelGreaterThan(LEVEL2, "Closing shared Library: %s\n", DLCloseData->GetPath()->c_str());
        delete DLCloseData;
    }
}
int RpnOperationsTable::m_FindAndAddPluginLibraries()
{
    int Found = 0;
    path plugins_dir(*m_SearchPath);
    path SharedLibExtention(".so");        // Change to .dll on Microsoft Windows
    if ((exists(plugins_dir)) && (is_directory(plugins_dir)))
    {
        for(auto& File_Iter : make_iterator_range(directory_iterator(plugins_dir), {}))
        {
            path PathToCheck = File_Iter;
            ShowOnlyIfLevelGreaterThan(LEVEL2, "Found library: %s\n", PathToCheck.c_str());
            if ((!is_directory(File_Iter)) && (PathToCheck.extension() == SharedLibExtention))
            {
                if (m_AddLibraryToTable(PathToCheck.string()))
                {
                    Found++;
                }
            }
        }
    }
    else
    {
        string Emsg = "The search path : ";
        Emsg.append(*m_SearchPath);
        Emsg.append(" either doesn't exist or is not a directory");
        throw runtime_error(Emsg);
    }
    return Found;
}
int RpnOperationsTable::m_AddLibraryToTable(string Library)
{
    int Added = 0;
    ObjectLevelDebuggingOrTesting("Attempting to insert library %s\n", Library.c_str());

    RpnDLData *DLCloseData = new RpnDLData(Library, GetObjectDebugLevel());
    if (!DLCloseData->IsLibraryOpen())
    {
        ShowOnlyIfLevelGreaterThan(LEVEL3, "Can't open shared library : %s\n", Library.c_str());
        return Added;    // If errors occur then ignore this library
    }
    const OpTableEntry *TableEntry = DLCloseData->GetOperationTableData();
    if (!TableEntry)
    {
        delete DLCloseData;
        ShowOnlyIfLevelGreaterThan(LEVEL3, "Can't find symbol rpnhub_plugin in  : %s\n", Library.c_str());
    }
    else
    {
        m_OpenedLibraries.push_back(DLCloseData);
        m_Operations[TableEntry->name] = TableEntry;
        Added++;
    }

    return Added;
}
void RpnOperationsTable::ExecuteOperation(string InputToken, stack<double>& Operands)
{
    OpTableEntry const *TableEntry = m_Operations[InputToken];
    if (TableEntry)
    {
        TableEntry->FuncPtr(Operands);
        ShowOnlyIfLevelGreaterThan(LEVEL2, "Performed : %s\n", InputToken.c_str());
    }
    else
    {
        char Operator[32];
        strncpy(Operator, InputToken.c_str(), 32);
        Operands.push(atof(Operator));
        ShowOnlyIfLevelGreaterThan(LEVEL2, "Added %s to Operands\n",
            InputToken.c_str());
    }
}
void RpnOperationsTable::TestReportThisObject()
{
    ObjectLevelDebuggingOrTesting("RPN Operations Table Object Test Report\n");
    ObjectLevelDebuggingOrTesting(
        "\tRpnOpsTable->TestLevel = %d\n", GetObjectTestLevel());
    ObjectLevelDebuggingOrTesting(
        "\tRpnOpsTable->DebugLevel = %d\n", GetObjectDebugLevel());
    ObjectLevelDebuggingOrTesting(
        "\tCurrent Path : %s\n", m_SearchPath->c_str());
    ObjectLevelDebuggingOrTesting(
        "\t%d Operations were added to the Operations Table\n",
        static_cast<int> (m_Operations.size()));

    for (auto& kv : m_Operations)
    {
        ObjectLevelDebuggingOrTesting("\t\tKey '%s' 0x%x\n",
            kv.first.c_str(), kv.second);
    }
}

RpnCalc.h

#ifndef RPNCALC_H_
#define RPNCALC_H_
#include "DbgTstHandling.h"
using namespace std;
class RpnCalculator : protected DebugAndTestHandling
{
private:
    class RpnOperationsTable *m_OpsTable;
    class RpnCalculatorIOSystem *m_IOSystem;
protected:
    inline int Test_DoesOperationsTableExist() {
        return ((m_OpsTable) ? 1 : 0);
    };
    inline int Test_DoesIOSystemExist() {
        return ((m_IOSystem) ? 1 : 0);
    };
    virtual void CalCulatorRunLoop();
public:
    RpnCalculator(int argc, char const * const argv[],
        int ObjectDebugTestLevel=DEFAULTOBJECTTESTLEVEL);
    virtual ~RpnCalculator();
    virtual void RunUntilQuit();
    inline int Test_InternalTestsPassed() {
        return (
            Test_DoesOperationsTableExist() &&
            Test_DoesIOSystemExist()
        );
    };
    void TestReportThisObject();
};

#endif /* RPNCALC_H_ */

Community · Accepted Answer · 2017-05-23 12:40:48Z

I see a number of things that may help you improve your code.

Don't abuse `using namespace std`

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. It's especially bad to put it into header files, so please don't do that.

Use standard Boost directory structure

In files such as RpnOpsTab.cpp, there are a number of included Boost headers, but unfortunately, they're not in the standard hierarchy. For example, the code currently includes these lines:

#include <error_code.hpp>
#include <range.hpp>
#include <filesystem.hpp>

but they should normally be written like this:

#include <boost/system/error_code.hpp>
#include <boost/range.hpp>
#include <boost/filesystem.hpp>

Use the newer style parametric constructors

In many of the class constructors, there is code like this:

TestBase::TestBase() {
    mA_ObjectMinimumLevel = DEFAULTOBJECTTESTLEVEL;
}

However the more modern style would be to write it like this instead:

TestBase::TestBase() 
    : mA_ObjectMinimumLevel{DEFAULTOBJECTTESTLEVEL}
{ }

Let the compiler create member functions

In TestBase the virtual destructor has no body and no effect. Rather than writing one manually, you could simply let the compiler write it for you.

virtual ~TestBase() = default;

Avoid explicitly using `this`

Many of the clases include explicit references to this that aren't really needed and just add to visual clutter. For example, instead of this:

inline unsigned int  mF_GetObjectLevel()
{
    return this->mA_ObjectMinimumLevel;
};

You can write this:

unsigned int mF_GetObjectLevel()
{
    return mA_ObjectMinimumLevel;
}

Note that I've omitted the inline keyword and the trailing ;, neither of which are necessary. The function is likely to be inlined anyway (the keyword is only a suggestion) and the semicolon is syntactically superfluous.

Use `const` where practical

Functions such as the above mentioned mf_GetObjectLevel() do not and should not alter the underlying object. For that reason, they should be declared const:

unsigned int mF_GetObjectLevel() const

Simplify naming

There is not really any useful reason to have mA_ or mF_ prefixes. I'd recommend using the common m_ prefix for member data items, and no prefix for functions.

Rethink your class design

The DebugBase and TestBase classes are very similar and the DebugAndTestHandling class inheirits from both. It may make more sense instead to have a single Log class and have the DebugAndTestHandling class contain two instances of it (one for test and one for debug). It would seem to simplify and rationalize the interface.

Avoid `new` and `delete`

Modern C++ uses new and delete and raw pointers much much less often than the version you and I both learned in the 1980s. For instance, within RpnOperationsTable::m_AddLibraryToTable() you could simply create the RpnDLData object to be added to the std::vector and rely on the object going out of scope to implicitly delete it.

int RpnOperationsTable::addLibraryToTable(std::string Library)
{
    int Added = 0;
    ObjectLevelDebuggingOrTesting("Attempting to insert library %s\n", Library.c_str());

    RpnDLData DLCloseData(Library, GetObjectDebugLevel());
    if (!DLCloseData.IsLibraryOpen())
    {
        ShowOnlyIfLevelGreaterThan(LEVEL3, "Can't open shared library : %s\n", Library.c_str());
        return Added;    // If errors occur then ignore this library
    }
    const OpTableEntry *TableEntry = DLCloseData.GetOperationTableData();
    if (!TableEntry)
    {
        ShowOnlyIfLevelGreaterThan(LEVEL3, "Can't find symbol rpnhub_plugin in  : %s\n", Library.c_str());
    }
    else
    {
        m_OpenedLibraries.push_back(DLCloseData);
        m_Operations[TableEntry->name] = TableEntry;
        Added++;
    }
    return Added;
}

Note that this change also assumes that the m_OpenedLibraries becomes an array of objects rather than an array of pointers. That is, it would be declared like this:

std::vector<class RpnDLData> m_OpenedLibraries;

This also simplifies other things considerably. For example, the routine to close all libraries can now be as simple as this:

void RpnOperationsTable::m_CloseAllLibraries()
{
    for (auto& OpenSharedLib : m_OpenedLibraries)
    {
        ShowOnlyIfLevelGreaterThan(LEVEL2, "Closing shared Library: %s\n", OpenSharedLib.GetPath()->c_str());
    }
    m_OpenedLibraries.clear();
}

Simplify class interfaces

Most of the instances in which the code calls the various logging functions, tend to look like this:

ShowOnlyIfLevelGreaterThan(LEVEL2, "Found library: %s\n", PathToCheck.c_str());

Note that the c_str() operation is called in very many intances and further, that there is no real prevention to passing an incorrect number of arguments. That is, this would also compile and run without complaint:

ShowOnlyIfLevelGreaterThan(LEVEL2, "Found library:\n", PathToCheck.c_str());

The difference is just that the "%s" is missing from the format string. I'd suggest instead something that supports operator << so that it could be written like this:

if (logLevel > LEVEL2) {
    log << "Found library: " << PathToCheck << '\n';
}

This also has the advantage that if the log level is such that it would not be printed anyway, that the rest of the statement is never evaluated. This may have a performance advantage.

Eliminate "magic numbers"

This code has a number of "magic numbers," that is, unnamed constants such as 32, 2048, etc. Generally it's better to avoid that and give such constants meaningful names. That way, if anything ever needs to be changed, you won't have to go hunting through the code for all instances of "32" and then trying to determine if this particular 32 means the maximum size of an operator or some other constant that happens to have the same value.

OK, let me see if I'm getting this correctly: Avoid the using namespace "???" statement to better identify where something is comming from, it prevents collisions of symbol names from diffe rent names spaces. To better identify the include files don't use the -I compilerswitch. Reduce the use of new and delete, and especially pointers, and depend on variables going out of scope rather than explicity deleteing them, question, doesn't RpnDLData DLCloseData go out of scope at the end of the m_AddLibraryToTable() function, or is the new variable a reference? — pacmaninbw
– pacmaninbw ♦, Commented Mar 5, 2016 at 23:14
You've got it. Yes DLCloseData goes out of scope, but note that what gets added to the std::vector is a copy of that object rather than a pointer in the revised scheme. — Edward
– Edward, Commented Mar 5, 2016 at 23:31

Stack Exchange Network

Reverse Polish notation calculator similar to dc on Unix/Linux systems using dynamic libraries

TstDbgCommon.h

TestBase.h

TestBase.cpp

DebugBase.h

DebugBase.cpp

DbgTstHandling.h

DbgTstHandling.cpp

RpnDLData.h

RpnDLData.cpp

RpnOpsTab.h

RpnOpsTab.cpp

RpnCalc.h

1 Answer 1

Don't abuse `using namespace std`

Use standard Boost directory structure

Use the newer style parametric constructors

Let the compiler create member functions

Avoid explicitly using `this`

Use `const` where practical

Simplify naming

Rethink your class design

Avoid `new` and `delete`

Simplify class interfaces

Eliminate "magic numbers"

You must log in to answer this question.

Linked

Hot Network Questions

Reverse Polish notation calculator similar to dc on Unix/Linux systems using dynamic libraries

TstDbgCommon.h

TestBase.h

TestBase.cpp

DebugBase.h

DebugBase.cpp

DbgTstHandling.h

DbgTstHandling.cpp

RpnDLData.h

RpnDLData.cpp

RpnOpsTab.h

RpnOpsTab.cpp

RpnCalc.h

1 Answer 1

Don't abuse using namespace std

Use standard Boost directory structure

Use the newer style parametric constructors

Let the compiler create member functions

Avoid explicitly using this

Use const where practical

Simplify naming

Rethink your class design

Avoid new and delete

Simplify class interfaces

Eliminate "magic numbers"

You must log in to answer this question.

Linked

Related

Hot Network Questions

Don't abuse `using namespace std`

Avoid explicitly using `this`

Use `const` where practical

Avoid `new` and `delete`