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

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

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 
#include 
#include 

but they should normally be written like this:

#include 
#include 
#include 

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 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 forma

StackExchange Code Review Q#121917, answer score: 2