-
Notifications
You must be signed in to change notification settings - Fork 35
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
Enforce const correctness #38
Comments
In GitLab by @valentjn on Nov 17, 2016, 15:00 My two cents:
Are there any specific code areas you thought about for adding |
In GitLab by @valentjn on Nov 17, 2016, 15:14 Oh yeah, and w.r.t. the "improve overall code quality without breaking any of the code which builds on top of it": That's only true if you're talking about adding That's just the thing about |
In GitLab by @lettrich on Nov 18, 2016, 18:08 Thank you for you for sharing your experiences and your opinion. I agree with the points you made - especially 4. I opened this "issue" more as a discussion/survey to see how much it matters to the main contributors and how much I should car as ultimately my goal is to write good code and help the project (especially parts of As far as adding |
In GitLab by @MLocs on Dec 14, 2016, 13:44 I think, to move this discussion forward, we need to consider individual parts. And I just happen to have a good example 😉 : the evaluation of the basis functions is symantically stateless and hence should not change the grid structure. I started to add const modifier to the function and then noticed that BsplineClenshawCurtisBasis evaluation actually does. @valentjn: I may be working on the outdated version of the code, is it something that you are using? Is the evaluation is logically const and the user does not see the changes? |
In GitLab by @lettrich on Dec 22, 2016, 14:30 @MLocs I am not entirely sure which member functions you are talking about but the concept sounds logical. Maybe my observation is related to this. I have noticed this for |
In GitLab by @valentjn on Dec 22, 2016, 14:42 Oops, sorry @MLocs, that one slipped through my attention (blame the many MR mails). No, it's not an outdated version of the code. I agree with both of you, the evaluation should logically be a |
In GitLab by @MLocs on Dec 23, 2016, 13:22 Yes, I addressed both of your concerns (I think) in my current branch as a side note to what I was currently doing 😉 |
In GitLab by @valentjn on Dec 23, 2016, 13:33 Ah--lo and behold! The endless depths of the many mysteries of C++ provide a solution for this very problem (usually the depths introduce even more mysteries). I did not know the Thanks and Merry Christmas Valeriy (I just saw I forgot it in the e-mail). |
In GitLab by @MLocs on Dec 23, 2016, 13:40 Merry Christmas to you too! 🎅 :mother_christmas: 🎄 🎁 |
In GitLab by @lettrich on Nov 17, 2016, 14:28
It is generally considered good practice to use the
const
whenever possible. All guidelines on C++ programming agree on that. Not only does it help to identify input and output parameters, it also helps to ensure that certain side effects do not occur which can be very hard to find. And the compiler enforces it automatically during compilation with zero runtime overhead. As Most interfaces insgpp::base
have already stabilized and we know which parameters are input and output parameters and which member functions actually change the state of of the object I would see it as a good measure to improve overall code quality without breaking any of the code which builds on top of it. What are the thoughts on that ?PS: I'd gladly help implement the necessary changes!
The text was updated successfully, but these errors were encountered: