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

Usage of noncopyable and -Weffc++ warnings #38

Open
rbraud opened this issue Aug 14, 2015 · 3 comments
Open

Usage of noncopyable and -Weffc++ warnings #38

rbraud opened this issue Aug 14, 2015 · 3 comments

Comments

@rbraud
Copy link

rbraud commented Aug 14, 2015

Hi, would it be possible to make the code compile cleanly with -Weffc++? Most of the warnings come from not initializing all class members with initializer lists. However, some of the warnings are about noncopyable. Your implementation doesn't actually prevent anyone from making a copyable object since the deleted copy constructor and assignment operator are only protected, meaning they can still be overridden by derived classes. The only way that I can see to remove every one of the warnings would be to declare these two methods in your classes as "delete" explicitly, I can live with the "has pointer member but does not override..." warnings, as it may not be worth the effort of removing noncopyable entirely. However, it would be really nice if all of the warnings about uninitialized members could be addressed. I can also provide a PR for this, just try it out first with -Weffc++ and let me know what you want to do about noncopyable.

@luca3m
Copy link
Owner

luca3m commented Aug 17, 2015

Hi,

I've changed a bit noncopyable to following some advices of Scott Mayers, like making public deleted methods. But I prefer to keep it because it's a nice DRY approach to make classes noncopyable.

About uninitialized members, my rule usually is to initialise only ones with non-default init, like integers or object that need special arguments. By the way is not a problem for me to have them on constructor too.

@rbraud
Copy link
Author

rbraud commented Aug 17, 2015

Putting those members public does not prevent people from making copies of those objects though. For example, you can declare a copy constructor in connection:

connection(const connection& other) : noncopyable(), _role(other._role), c(other.c) {
}

This compiles just fine. You can also do similar things with the operator=, and then you can copy these objects.

@luca3m
Copy link
Owner

luca3m commented Aug 17, 2015

Yes I know. We can make those classes final if this fixes, or leave the warning. It's not an error, noncopyable is used correctly to avoid copying.

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

No branches or pull requests

2 participants