Skip to content

Add property tests to resistor-color-duo#907

Closed
tejasbubane wants to merge 1 commit intoexercism:masterfrom
tejasbubane:resistor-duo-property-tests
Closed

Add property tests to resistor-color-duo#907
tejasbubane wants to merge 1 commit intoexercism:masterfrom
tejasbubane:resistor-duo-property-tests

Conversation

@tejasbubane
Copy link
Copy Markdown
Member

Closes #867

@tejasbubane tejasbubane force-pushed the resistor-duo-property-tests branch from c5b4115 to d70c7ac Compare April 19, 2020 10:40
@sshine
Copy link
Copy Markdown
Contributor

sshine commented Apr 20, 2020

Hi @tejasbubane, and thanks for your PR!

I'll get back to it in the next few days unless someone else does. :-)

Copy link
Copy Markdown
Contributor

@sshine sshine left a comment

Choose a reason for hiding this comment

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

Nice work so far.

I've left some comments below for improvement.

For interesting reading material,

https://www.fpcomplete.com/blog/quickcheck-hedgehog-validity

It may also be worth linking to in the link collection in your hint.

Comment on lines +14 to +15
instance Arbitrary Color where
arbitrary = oneof $ map return [Black, Brown, Red, Orange, Yellow, Green, Blue, Violet, Grey, White]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Three things:

Suggested change
instance Arbitrary Color where
arbitrary = oneof $ map return [Black, Brown, Red, Orange, Yellow, Green, Blue, Violet, Grey, White]
newtype AnyColor = AnyColor Color deriving (Show, Eq)
instance Arbitrary AnyColor where
arbitrary = AnyColor <$> elements [minBound..]

But instead of using the Arbitrary type class, consider simply making a colorGen:

Suggested change
instance Arbitrary Color where
arbitrary = oneof $ map return [Black, Brown, Red, Orange, Yellow, Green, Blue, Violet, Grey, White]
colorGen :: Gen Color
colorGen = elements [minBound..]

I might also place this generator at the bottom of the file.

@@ -1,19 +1,32 @@
{-# OPTIONS_GHC -fno-warn-type-defaults #-}
{-# OPTIONS_GHC -fno-warn-orphans #-}
Copy link
Copy Markdown
Contributor

@sshine sshine Apr 21, 2020

Choose a reason for hiding this comment

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

See comment below above.

Suggested change
{-# OPTIONS_GHC -fno-warn-orphans #-}

Comment thread exercises/resistor-color-duo/test/Tests.hs
Comment thread exercises/resistor-color-duo/test/Tests.hs
that only accepts two colors and produces the resistor's numeric value
component of its resistance.

**Bonus - Property Testing:**
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I haven't seen that we use "Bonus" or mark sub-subsections using **...** elsewhere on the track.

I might skip this heading altogether.

Comment thread exercises/resistor-color-duo/.meta/hints.md
Comment thread exercises/resistor-color-duo/.meta/hints.md
Comment thread exercises/resistor-color-duo/.meta/hints.md
Comment thread exercises/resistor-color-duo/.meta/hints.md
@sshine
Copy link
Copy Markdown
Contributor

sshine commented May 25, 2020

Hey @tejasbubane, are you interested in following up on this PR?

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.

resistor-color-duo and property tests

2 participants