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

Avoid the use of macros as Expected #35

Closed
viboes opened this issue Sep 24, 2015 · 6 comments
Closed

Avoid the use of macros as Expected #35

viboes opened this issue Sep 24, 2015 · 6 comments
Labels

Comments

@viboes
Copy link

viboes commented Sep 24, 2015

Having macros that are not prefixed by the library name is a bad practice as in

#define Expects(x)  Guide::fail_fast_assert((x))
#define Ensures(x)  Guide::fail_fast_assert((x))

Usually we use UPPERCASE letters for macros.

In addition, we have inline functions in C++, so no need for a macro that call another function ;-)

@gdr-at-ms
Copy link
Member

The Expects() and Ensures() come from the guidelines. Ideally, we should have language support for them in form of contracts (see my CppCon talk). In the meantime, the fact that they are macros is an implementation detail, and should not be exposed as al uppercase letters.

@viboes
Copy link
Author

viboes commented Sep 24, 2015

Do you mean that C++17/20 will have Expects and Ensures as keywords?
Introducing these macros prevents a user having such names in their application to use GSL, which is bad IMHO.

But why do you use a macro when an inline function is enough? It is so hard to read

gsl::expects(x);
gsl::ensures(x);

?

@viboes
Copy link
Author

viboes commented Sep 29, 2015

I would not label an issue as resolved when in reality it has been rejected.

@neilmacintosh
Copy link
Collaborator

Fair enough. Fixed.

@robindegen
Copy link

I agree with @viboes here. I have 2 questions; 1. why is it a macro and 2. why does it not follow the coding standard of the rest of the library with regards to capitalization?

@neilmacintosh
Copy link
Collaborator

  1. It is a macro so it can collect file/line information as helpful diagnostics when failures occur.
  2. The capitalization follows the name suggested by the Core Guidelines. To be honest, that is something that does not particularly bother me either way. There is a standardization proposal to add a contract system to C++. That does use different capitalization so maybe it will change in time.

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

No branches or pull requests

4 participants