-
Notifications
You must be signed in to change notification settings - Fork 17
Exercise review comments #6
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
Comments
Hi @noelmarkham Thanks for the feedback. I've created a PR that address a couple of suggestions ( Also, I've created an issue to improve the Grouping Properties section. We need to figure out how to improve the exercise and let the user introduce some value. See #8 Related with the three first bullets and the lang review, practically all texts has been taken from the User Guide but we could improve them. Please, feel free to create any PRs or leave me more suggestions, any help will be very appreciated. Thanks |
Ok, regarding the first three bullets:
|
As discussed, my comments from these exercises. I think they're really good.
Properties
sqrt
function returnsDouble
, notInt
, even though (for example)9.0 == 9
. But the property provided does not work for negative numbers. While I understand this is only an example, and thus it doesn't matter if the property holds or not, I would suggest it would be clearer if the property holds, and is unambiguous.(n == 0) ==> (n == 0)
) is a bit confusing. I think it would be better if the implication was different to the property.forAll
s at the same indent level, as I thought these were nested.ZeroSpecification
class takes a parameter, which (I think) is only ever0
. I think it would just be clearer if thevalue
parameter is removed and simply replaced with0
.Generators
{
and(
on thecheck
methods.I can't think of any headline items to add, perhaps you may want to show a property that actually fails, and perhaps how to label properties and generators.
I've not done a language review on this either, please let me know if you want me to do that.
I'll leave these suggestions to you, but I'm happy to help if you need it.
The text was updated successfully, but these errors were encountered: