Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Offer alternative to FOR_STAT_ARRAY? #4

Open
mspraggs opened this issue Nov 11, 2015 · 2 comments
Open

Offer alternative to FOR_STAT_ARRAY? #4

mspraggs opened this issue Nov 11, 2015 · 2 comments
Assignees

Comments

@mspraggs
Copy link
Contributor

General advice I've read suggests that macros should be avoided where possible in C++ as it makes debugging harder. With that in mind, could we offer an alternative to FOR_STAT_ARRAY in LatAnalyze3?

One idea I had would be to simply offer a member function in the Sample class that accepts a lambda function that accepts an index, say, then runs that function under a for loop, as is currently coded in FOR_STAT_ARRAY.

@aportelli
Copy link
Owner

In C++ one indeed like to avoid macros because they are just text processing and I tend to agree with this philosophy. However, as far as FOR_STAT_ARRAY is concerned I would like to keep it like that for the moment. Here are some arguments I can think about:

  1. Macros are hard to debug if they are on multiple lines because some compiler will just indicates the line where the macro is used but not the line in the macro where the problem is. However FOR_STAT_ARRAY is a one-liner so there won't be any ambiguity on where the problem came from. Moreover, modern compilers (e.g. clang/gcc 5) are more and more verbose on macro errors and in my experience it is less a struggle than in the past.
  2. With the macro the loop body is aware of everything which in the higher scope. With the lambda function you will in some case end up with a complicated capture code. Maybe the code is semantically stronger but I am not sure the readability will be as good as in the macro case.
  3. I think unaryExpr and binaryExpr members to Eigen types already implement something similar to what you describe. One difference if that this works with the expression template, i.e. it won't evaluate anything before an assignment takes place.
  4. For me a real nice substitute too loop over elements would be to have a C++ iterators, which would allow us to use the C++11 'for range' loops. However, my classes inherit from Eigen classes and I believe this is where the change should be made. There is a discussion in progress since quite some time http://eigen.tuxfamily.org/bz/show_bug.cgi?id=231 so I have some hope the Eigen guys will do it soon.

@mspraggs
Copy link
Contributor Author

All good points. I didn't know Eigen had this functionality. I'll investigate. For me the disadvantage of the 'for range' structure is that if you're assigning from one array to another (as we're generally doing here), there's no easy way to relate the elements of one array to another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants