tschoonj / xraylib

A library for X-ray matter interaction cross sections for X-ray fluorescence applications
https://github.com/tschoonj/xraylib/wiki
Other
120 stars 54 forks source link

Simplified macro in cplusplus/xraylib++.h #144

Closed mschollmeier closed 3 years ago

mschollmeier commented 3 years ago

I've simplified the macro in cplusplus/xraylib++h and added a test file to check for correct returns and error messages.

tschoonj commented 3 years ago

Thanks for the PR! That looks really nice.

One thing that doesn't work is passing std::string instances to those functions that deal with chemical compounds. They appear to accept only C-strings.

For example this doesn't compile:

    res = xrlpp::Refractive_Index_Re(std::string("H2O"), 1, 1);

Would it be possible to add support for this? Apologies, but I am not much of a C++ coder, and I have always avoided having to use templates 😄

mschollmeier commented 3 years ago

I'm also not an expert in templates, but this looked like a low-hanging fruit.

Thanks for pointing out this issue. AFAIK, std::string does not work because the compiler pushes that directly to the C function.

There seems to be no straightforward way to filter for std::string without (complicated) partial template specializations. We could do the following:

  1. use a 2nd macro for those functions that have a string as their first input argument (see below)
  2. leave as-is and ask the programmer to do the c_str conversion in the function call. E.g., res = xrlpp::Refractive_Index_Re(std::string("H2O").c_str(), 1, 1);
#define _XRL_FUNCTION_S(_name) \
    template<typename... V> \
    double _name(const std::string arg1, const V... args) { \
        xrl_error *error = nullptr; \
        double rv = ::_name(arg1.c_str(), args..., &error); \
        _process_error(error); \
        return rv; \
    }
tschoonj commented 3 years ago

This seems to fix things:

diff --git a/cplusplus/tests/test-xrl_functions.cpp b/cplusplus/tests/test-xrl_functions.cpp
index 55357e2..6a3e291 100644
--- a/cplusplus/tests/test-xrl_functions.cpp
+++ b/cplusplus/tests/test-xrl_functions.cpp
@@ -265,6 +265,8 @@ int main(int argc, char **argv) {
         assert(strcmp(e.what(), NEGATIVE_DENSITY) == 0);
     }

+    res = xrlpp::Refractive_Index_Re(std::string("H2O"), 1, 1);
+
     /* 1 string, 3 double */
     res = xrlpp::DCSP_Rayl_CP("FeSO4", 10., 1.5707964, 3.14159);
     assert(fabs(res - 3.59519e-13) < 1.0e-18);
diff --git a/cplusplus/xraylib++.h b/cplusplus/xraylib++.h
index ffff561..76459e6 100644
--- a/cplusplus/xraylib++.h
+++ b/cplusplus/xraylib++.h
@@ -26,6 +26,13 @@ using _compoundDataNISTPod = struct compoundDataNIST;

 // Macro to be used below
 #define _XRL_FUNCTION(_name) \
+    template<typename... T> \
+    double _name(const std::string &compound, const T... args) { \
+        xrl_error *error = nullptr; \
+        double rv = ::_name(compound.c_str(), args..., &error); \
+        _process_error(error); \
+        return rv; \
+    } \
     template<typename... T> \
     double _name(const T... args) { \
         xrl_error *error = nullptr; \

I tried committing this and pushing to your branch but that wasn't possible, probably because you used your master branch for this PR.

tschoonj commented 3 years ago

I will merge this in as is. There is a problem with Homebrew at the moment, but I don't doubt that the tests would also pass on macOS.

Many thanks @mschollmeier !!