Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork130
Made WCharacter.h more clear#26
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
a == 0 ? false : true is same as a != 0Also added extern C
CLAassistant commentedApr 9, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
|
| @@ -22,6 +22,12 @@ | |||
| #include <ctype.h> | |||
| // This Mentions that the following code is | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is useless. It only changes how the function names are mangled. Since the functions are not compiled into an object file nor a static library, it doesn't matter if they're extern "C" or not. They're inline, in C++ and in C.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't think this is true for anyinline function, since they are not necessarily inlined. In this case, they arealways_inline so it should indeed not matter, but it might be good to still have theextern "C" for good measure (and in case other non-always_inline functions are added later). I would omit the comment here, though, since the meaning/intention ofextern "C" is rather obvious (and can be looked up if not) and I believe it's used without comment elsewhere too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks like a reasonable change to me. As@leg0 suggests,extern "C" is not strictly required, but it doesn't hurt. I can imagine removing the comment about it, though. See also inline comment.
| @@ -22,6 +22,12 @@ | |||
| #include <ctype.h> | |||
| // This Mentions that the following code is | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I don't think this is true for anyinline function, since they are not necessarily inlined. In this case, they arealways_inline so it should indeed not matter, but it might be good to still have theextern "C" for good measure (and in case other non-always_inline functions are added later). I would omit the comment here, though, since the meaning/intention ofextern "C" is rather obvious (and can be looked up if not) and I believe it's used without comment elsewhere too.
Hi@leg0 👋 Can you please sign the CLA? |
@aentinger I have already signed it. |
We need@pratikpc to sign the CLA: |
Mea culpa 😊 |
@pratikpc can you please sign it while being logged in via the right email ... likely the email stored in the commit is different from the one you are signed in GitHub. |
It's obvious that
a == 0 ? false : trueis same asa != 0Also a != 0 looks more clear as a Return Type to a bool function.
It's more easier to understand what it does in this form.
WCharacter.h was full of functions which utilised
a == 0 ? false : trueinstead of the more easy to reada != 0which strikes a fine balance between simplicity and readability.If we are to look at Standard Library Definitions of the various functions used like isdigit,
isalphaetc. we find they return 0 when they do not find anything thus makingisdigit(ch) != 0even more clearerAdded extern C
Given the fact that we are using C Functions and code that is compatible with the C Standard, I added extern C. Extern C is also used in similar C Standard Compatible Arduino header files