Skip to content
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

Port test suite so it supports Data.Text #841

Open
4 of 10 tasks
sshine opened this issue Sep 4, 2019 · 5 comments
Open
4 of 10 tasks

Port test suite so it supports Data.Text #841

sshine opened this issue Sep 4, 2019 · 5 comments

Comments

@sshine
Copy link
Contributor

sshine commented Sep 4, 2019

In #780, exercises that might be applicable for teaching the Data.Text module were found. In some of those cases, the test suite was extended with OverloadedStrings so that both String and Text solutions work, and an example solution was provided. For a notable example of this, one of the early exercises, Bob, had an example added, its test suite updated, and had the following hint added. (See 28cc7d5.)

This issue contains the remaining exercises for which Data.Text may be applicable. Contributors are free to pick an exercise they're comfortable with, (1) make an example solution, (2) make the test suite compatible with multiple string types, (3) add a hint, and optionally (4) make Data.Text the default in the stub file.

List of exercises that don't yet support Data.Text but could:

  • rotational-cipher: Easy.
  • raindrops: Easy.
  • diamond: Easy.
  • twelve-days: Easy.
  • atbash-cipher: Easy.
  • run-length-encoding: Doable.
  • ocr-numbers: Doable.
  • rail-fence-cipher: Doable.
  • luhn: Easy, but hardly relevant as the exercise mainly consists of working on integers.
  • series: Easy. Test suite already contains OverloadedStrings and already has ByteString -> Seq (Seq a). So a Data.Text example is probably not necessary unless the stub should encourage something other than String by default.

I've looked at the example solution(s) for each of these and made a quick estimate where "Easy" means that it would be quick to translate this to Data.Text by finding equivalent combinators, and "Doable" means that the contributor probably needs to learn (part of) the algorithm in the exercise to port it. The estimation was made very quickly and subjectively, so your mileage may vary.

@chiroptical
Copy link
Contributor

I can add Data.Text support to diamond. I am working my way through the exercises and I don't want to contibute to ones I haven't completed. That being said, the next closest exercises are raindrops and twelve-days.

@sshine
Copy link
Contributor Author

sshine commented Sep 8, 2019

@barrymoo: That sounds good! Since you're adding property-based tests for Diamond in #843 right now, I would recommend that you either (1) wait with adding Data.Text support until after, and submit a separate PR, or (2) add support for Data.Text in a feature branch forked from master, submit a separate PR, and once this is merged, rebase #843 onto it.

I would recommend (1) because it involves fewer merge conflicts.

tejasbubane added a commit to tejasbubane/haskell that referenced this issue Oct 3, 2019
tejasbubane added a commit to tejasbubane/haskell that referenced this issue Oct 3, 2019
tejasbubane added a commit to tejasbubane/haskell that referenced this issue Oct 3, 2019
tejasbubane added a commit to tejasbubane/haskell that referenced this issue Oct 9, 2019
tejasbubane added a commit to tejasbubane/haskell that referenced this issue Oct 9, 2019
sshine pushed a commit that referenced this issue Oct 10, 2019
sshine pushed a commit that referenced this issue Oct 21, 2019
Addresses #841.

Because the Diamond exercise uses property-based tests, it is not enough to 
assume that an instance of `IsString s` also has `Eq s`, since the properties
use several string-related functions. And because `String` is polymorphic and
`Text` is monomorphic, there is no common interface to these functions.

This os overcome with the 'string-conversions' package.
@chiroptical
Copy link
Contributor

I will work on twelve-days next.

@moniquelive
Copy link

Could you consider wordy as well?

TIA

@sshine
Copy link
Contributor Author

sshine commented Nov 29, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants