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

enh: using namespace std in header #13

Open
gabywh opened this issue Jul 28, 2021 · 0 comments · May be fixed by #14
Open

enh: using namespace std in header #13

gabywh opened this issue Jul 28, 2021 · 0 comments · May be fixed by #14

Comments

@gabywh
Copy link

gabywh commented Jul 28, 2021

Enhancement:

I appreciate in a 2-file project it's probably not so much an issue but you really shouldn't be putting using namespace std in a header file as this forcibly includes the whole std namespace for any implementation code that wishes to use the bloom.hpp header.

Avoiding placing using namespace std (or indeed any using namespace X) in a header is mentioned in so many places I forget where - maybe Exceptional C++ - / Sutter would be one, Scott Meyers' Effective STL almost certainly another (Bjarne Stroustrup C++ Prog Lang another?) - but one ref I did just find an old classic from Sutter's GOTW: http://gotw.ca/gotw/053.htm see Section 2, Guideline #1:

  1. Using directives should be avoided entirely, especially in header files

Sutter also makes the distinction here - as I did above - in a footnote that this guideline is valid for shared header files (i.e. used by more than one cpp) : http://gotw.ca/gotw/053.htm#1 so you could argue "no change here" since only one cpp, but then you could also argue that it is simply poor style - which I why I label this as an enhancement

BTW: nothing against using namespace std in your cpp, should you wish to do so - so this is probably a one-line change: remove using namespace std from the header and add it to the cpp. Depending on where you place the using in your cpp, you may need to fully qualify any STL types in the header file.

I can fork this repo and supply a PR with change if you accept this as an (enhancement) issue.

@gene-hightower gene-hightower linked a pull request Oct 14, 2022 that will close this issue
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

Successfully merging a pull request may close this issue.

1 participant