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

Update c++ example to use c++11 features, std lib, and error handling #135

Closed mschollmeier closed 3 years ago

mschollmeier commented 3 years ago

Updated code for #134

mschollmeier commented 3 years ago

If c++ checks fails, please add -std=gnu++11 to the compiler options

tschoonj commented 3 years ago

I think you will need to include <memory> explicitly. Pretty sure that the CentOS 8 compiler already works in C+11 mode.

mschollmeier commented 3 years ago

Looks like the CentOS compiler is not using c++11. I get the same error messages without the -std=gnu++11 switch on my Mac

tschoonj commented 3 years ago

Since I don't want to do this with a flag just for the CentOS 7 build, I will add some logic to the autotools script for this. But that won't be for tonight, perhaps later this weekend or on Monday.

mschollmeier commented 3 years ago

If the Xcode built also fails, here are my build commands on Catalina with Xcode 11.3.1. The -isysroot <path> switch was necessary for clang to find math.h.

/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -std=gnu++11 -stdlib=libc++ -I/usr/local/xraylib/include/xraylib -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk -L/usr/local/xraylib/lib -lxrl xrlexample6.cpp

tschoonj commented 3 years ago

Hi @mschollmeier

As you can see I made some small changes that allowed for the CI builds to pass.

If you're happy with this, I will merge it in.

This being said, if I had to use xraylib in C++ nowadays, I would use a method that would map the different error codes in xrl_error to the corresponding exception classes in the C++ standard library. For example an error with code XRL_ERROR_INVALID_ARGUMENT would result in an invalid_argument exception being thrown.

This would mimic the behaviour shown by the other language bindings.

mschollmeier commented 3 years ago

@tschoonj Thanks for the fixes. I like the idea of translating error codes, I'll implement something in the next few days.

tschoonj commented 3 years ago

Actually how about writing a header only version of xraylib for C++?

Something like:


#include <xraylib.h>

namespace xrlpp {

    void process_error(xrl_error *error) {
      if (!error)
        return;
      switch (error->code) {
        case XRL_ERROR_INVALID_ARGUMENT:
          throw std::invalid_argument(error->message);
        ...
      }
    }

    int LineEnergy(int Z, int line) {
     xrl_error *error = NULL;
     double rv = ::LineEnergy(Z, line, &error);
     process_error(error);
     return rv;
   }

}

Most of this file could be generated using a Python script, similar to what I do for the Fortran and Python bindings, and it would also be nice to have class equivalents of struct compoundData etc that 'feel' more native to C++ programmers.

What do you think?

mschollmeier commented 3 years ago

I'm not sure if I understand correctly what you mean by header only. You would still need to link to libxrl, or? Other than that I like the idea of adding the namespace and rewriting so that xraylib takes care of throwing errors.

tschoonj commented 3 years ago

Yes, linking to libxrl would still be required, but there wouldn't be a separate shared library for the C++ code.

Is this something you would like to get involved in? I will be busy with work for the next couple of weeks but should be able to help out now and then.

mschollmeier commented 3 years ago

I can take a stab at it. I've just started including xraylib in my project, so this is a good time for that. This project is not my main job, so it'll take a few weeks. Other than the code sample above, how do you suggest implementing this?

tschoonj commented 3 years ago

The wrappers for the simple functions can be automatically generated with a python script, just like I do for Pascal and Fortran, whose folders contain a generate-code.py script for this purpose.

A separate header, which includes the auto-generated one, would need to be written by hand and will contain the process_error method as well as all methods that deal with the compoundData, compoundDataNIST and radioNuclideData structs.

There's absolute no urgency here... do feel free to take your time.

lgtm-com[bot] commented 3 years ago

This pull request introduces 1 alert when merging 00217abd4f824da5e0f89da95d4ba1dfb2703155 into 862c8ab9d1f45f2a03ca925ec62ffba1bf289d92 - view on LGTM.com

new alerts:

mschollmeier commented 3 years ago

How did that happen? I did not intend to push into your master?!

tschoonj commented 3 years ago

Don't worry, you didn't do that. All you did was pull my updated master branch into your current branch.

tschoonj commented 3 years ago

Closed in favour of #139