Skip to content

Add parentheses, remove unused variables, replace abs by fabs#2

Merged
edisongustavo merged 3 commits into
ESSS:masterfrom
michael-hartmann:master
Nov 23, 2016
Merged

Add parentheses, remove unused variables, replace abs by fabs#2
edisongustavo merged 3 commits into
ESSS:masterfrom
michael-hartmann:master

Conversation

@michael-hartmann
Copy link
Copy Markdown
Contributor

This PR includes three changes:

  1. Add parenthesis around && operator: This improved the readability of the code and prevents a compiler warning with gcc.
  2. Remove unused variables.
  3. Use fabs instead of abs in dqagi.c: This might have been even a bug. The abs function computes the absolute value of an integer. So, here the double will be converted to an integer and the absolute value of the integer will be computed. I think the right behavior is to compute the absolute value of the double.

The abs function computes the absolute value of an integer only. Replace abs by
fabs to calculate the absolute value of a double.
The && operator has higher precedance than the || operator. Therefore, this
commit doesn't change the behavior of the code. However, this commit makes the
code easier to understand and it prevents some compiler warnings (-Wparentheses
with gcc).
Copy link
Copy Markdown
Contributor

@edisongustavo edisongustavo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

You're correct that the usage of abs() is error-prone.
I agree that using fabs() is safer and not error prone (like abs()).

It is not wrong at the actual state since abs() is overloaded for double types at math.h. So it is not currently a bug.

It is a healthy improvement to the code nonetheless.

@edisongustavo edisongustavo merged commit 4192d7e into ESSS:master Nov 23, 2016
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 this pull request may close these issues.

2 participants