[MOAB-dev] r4016 - MOAB/trunk/tools/dagmc

[Jason Kraftcheck]

Paul Wilson wrote:
> Alas, I find myself more convinced of the others' view.  
> 
> It seems dangerous for the public interface to depend on a define, thus
> allowing the library to be built with one interface and the calling app
> to be built expecting another.  (Perhaps in an earlier email to me)
> Steve pointed out that moab.make is more of a convenience than a
> requirement for building apps with MOAB.  We could include relevant
> defines in moab.make (one way or another) but it still allows for the
> possibility that this inconsistency arises when a developer decides not
> to use moab.make. 
> 
> Furthermore, it was a particularly difficult inconsistency to track down
> as there were no compile-, link-, or runtime errors - just the wrong
> answer when the code ran!
> 
> Note, also, that CGM defines are still available after this fix, they
> just don't change the public interface.
> 
> Some other ideas:
> 
>     * Modify the header (eg. DagMC.hpp) at install-time to install a
>       header that is unconditionally the same as the conditions used in
>       building MOAB.  That is, parse the header at install time with the
>       same macros as used to build so that the interface is consistent
>       and unambiguous

This can be done at configure time rather than install time using
AC_CONFIG_HEADERS.  If you choose to go this route then I recommend a)
putting the configured #define(s) in a separate header rather than DagMC.hpp
and including the configured header in DagMC.hpp and b) choosing a more
'namespaced' macro such as MOAB_CONFIG_CGM or DAGMC_CONFIG_CGM.

>     * <begin kludge (I'm brainstorming here)>Declare a function
>       prototype and/or invoke a function within the conditional code
>       that will at least raise a link-time error when the inconsistency
>       arises</kludge>
> 

More ideas:

a) In this case, the use of #ifdef CGM is entirely unnecessary.  The code
will compile and run fine with a forward declaration of the RefEntity class
and a declaration of a vector<RefEntity*> even if the RefEntity header is
never available and the code is never linked with CGM.  You only need the
actual RefEntity implementation (header and cgm lib) if you want to
dereference the pointers.

b) Use a variation of the PIMPL pattern to remove such implementation
details from the API (in this case the DagMC header) entirely.  For example:

class DagMCImpl;
class DagMC {
public:
...
private:
DagMCImpl* impl;
};

Put all of the private functions and all of the data in the DagMCImpl class.
 The definition of that class should never need to be part of the API
because it is only accessed by public members of DagMC.  In fact, the
declaration of the class should probably be in DagMC.cpp.

- jason