Port raindrops test-suite to support Data.Text

[tejasbubane]

Issue: #841

[sshine]

Excellent!

• The hint that was copied from Bob should mention convert rather than responseFor.
• The parenthesis around fromString expected will satisfy HLint. (CI fails here.)
• Running configlet fmt . --only raindrops will ensure that README.md is updated. (CI fails here.)

The rest is optional.

[sshine]

Suggested change

	- You can now write e.g. `responseFor :: Text -> Text` and refer to `Data.Text` combinators as e.g. `T.isSuffixOf`.
	- You can now write e.g. `convert :: Int -> Text` and refer to `Data.Text` combinators as e.g. `T.pack`.

[sshine]

Come to think of it, perhaps this is superfluous with the "You can then replace ..." sentence in a bullet below.

Suggested change

	- You can now write e.g. `responseFor :: Text -> Text` and refer to `Data.Text` combinators as e.g. `T.isSuffixOf`.
	- You can now refer to `Data.Text` combinators as e.g. `T.pack`.

[sshine]

I like that guards are moved to the second line to reduce line length:

sound noise factor
  | n `rem` factor == 0 = Just noise
  | otherwise           = Nothing

Another way to write this is:

sound noise factor = do
  guard (n `rem` factor == 0)
  pure noise

The last line is incidentally what an octogenarian would say if you play dubstep to them.

[sshine]

After having reviewed #843, I've come to think that if-then-else solutions like

sound noise factor =
  if n `rem` factor == 0
  then pure noise
  else empty

are perfectly acceptable.

[sshine]

To make this version-control friendly,

maybeSound = sound "Pling" 3
   `mappend` sound "Plang" 5
   `mappend` sound "Plong" 7

Alternatively, use (<>), but this also looks good.

[sshine]

Also, this is a very cool use of the Semigroup a => Semigroup (Maybe a) instance!

[sshine]

Even though this is not a consistent standard in this track, or any recommended practice outside of this track, I've mostly seen that the module being tested is listed outside of alphabetical order and without indentation. Whether we should keep this convention or not is clearly debatable, but it should be outside the scope of this PR.

$ ls exercises
114
$ find . -name Tests.hs -exec ack '^import (?!Test)\S+ \(' {} \; | wc -l
100

Suggested change

	import Raindrops (convert)
	import Raindrops (convert)

[sshine]

CI throws up about this.

Suggested change

	assertion = convert number `shouldBe` (fromString expected)
	assertion = convert number `shouldBe` fromString expected

[sshine]

You could also avoid the maybeSound binding by writing

convert n = fromMaybe (T.pack (show n)) $
  sound "Pling" 3 <>
  sound "Plang" 5 <>
  sound "Plong" 7
  where
    sound = ...

[tejasbubane]

@sshine Thanks 👍 I have made the changes.

[sshine]

Excellent.

By the way, when you force push, I cannot see the difference since the last review.

When you don't, we can still squash merge the PR.

[petertseng]

Well, "cannot" is a flexible term. There's always https://github.com/exercism/haskell/compare/b33ff3ea9c0b44398d9ea8c199af58e061bcbcd3..79a5a2b05383911d49680351a0a1212af98f8110.

And may I point out that (on the web interface), the word "force-pushed" is a link to that.

[sshine]

Wow, I didn't know that was a hyperlink.

Thanks for that.