Skip to content

operator bool() and set_zero(): test for zero and set to zero respectively.#39

Merged
jzmaddock merged 6 commits intoboostorg:developfrom
jeremy-murphy:polynomial_zero
May 26, 2016
Merged

operator bool() and set_zero(): test for zero and set to zero respectively.#39
jzmaddock merged 6 commits intoboostorg:developfrom
jeremy-murphy:polynomial_zero

Conversation

@jeremy-murphy
Copy link
Copy Markdown
Contributor

Checking for zero or non-zero and setting to zero are common operations, so fast implementations are useful.

Checking for zero or non-zero and setting to zero are common operations,
so fast implementations
Comment thread include/boost/math/tools/polynomial.hpp Outdated
}

// Useful for fast test of equality to zero.
operator bool() const
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This should be explicit when BOOST_NO_CXX11_EXPLICIT_CONVERSION_OPERATORS is not set, otherwise use an implicit conversion to an "unmentionable" type, for example multiprecision::number uses:

typedef bool (self_type::*unmentionable_type)()const;

BOOST_MP_FORCEINLINE operator unmentionable_type()const
{
return is_zero() ? 0 : &self_type::is_zero;
}

Which prevents accidental conversion to an arithmetic type.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ahh, wow, that's a new one on me. What about naming a function that sets the polynomial to zero? I have used the verb clear but maybe set_zero is more appropriate? This is on the assumption that it is more efficient than assignment from zero, which I assume the compiler is not quite smart enough to optimize away for a class such as polynomial.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

On 12/05/2016 12:08, Jeremy W. Murphy wrote:

In include/boost/math/tools/polynomial.hpp
#39 (comment):

@@ -455,6 +454,18 @@ class polynomial :
normalize();
return *this;
}
+

  • // Useful for fast test of equality to zero.
  • operator bool() const

Ahh, wow, that's a new one on me. What about naming a function that
sets the polynomial to zero? I have used the verb |clear| but maybe
|set_zero| is more appropriate? This is on the assumption that it is
more efficient than assignment from zero, which I assume the compiler
is not quite smart enough to optimize away for a class such as polynomial.

clear() works for me.

@jeremy-murphy
Copy link
Copy Markdown
Contributor Author

I still need to update the docs after we settle on the names.

@jeremy-murphy
Copy link
Copy Markdown
Contributor Author

And what about the horrible workaround for gcc 4.6.x?

@jzmaddock
Copy link
Copy Markdown
Collaborator

On 12/05/2016 12:10, Jeremy W. Murphy wrote:

And what about the horrible workaround for gcc 4.6.x?

I don't believe that's required in this case - it was the presence of
other operator() overloads that caused the issue (it called the bool
operator rather than the correct one if memory serves).

@jeremy-murphy jeremy-murphy changed the title operator bool() and clear(): test for zero and set to zero respectively. operator bool() and set_zero(): test for zero and set to zero respectively. May 23, 2016
@jeremy-murphy
Copy link
Copy Markdown
Contributor Author

jeremy-murphy commented May 23, 2016

John, if you're happy with my choice of set_zero as the name for the modifying function, then this is all done from my end.

@jzmaddock
Copy link
Copy Markdown
Collaborator

Can you mark operator bool in the docs as explicit to match the code?

Re the name "set_zero", I confess I'm not overly keen on set_ and get_ prefixes, so I would be inclined to go with either just "zero" or even "clear". Does assigning from a literal zero produce the same result BTW?

Thanks for this, John.

@jeremy-murphy
Copy link
Copy Markdown
Contributor Author

I'm definitely not in favour of the object-oriented habit of 'getters and setters' on a per-member basis, but I thought it was OK in this case. I thought "clear" was too tied to the underlying implementation and is not meaningful to a polynomial per se (although that's not a huge sin). "zero" seems ambiguous as to whether it is setting or querying? I don't love "set_zero" but the alternatives all seem more problematic.

@jzmaddock
Copy link
Copy Markdown
Collaborator

OK lets go with what we have and move on, merging to develop...

@jzmaddock jzmaddock merged commit f8ee91a into boostorg:develop May 26, 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