Skip to content

Update test + example to use MakeWatt - #288

Closed
vmpstr wants to merge 1 commit into
purescript-contrib:masterfrom
vmpstr:makewatt
Closed

vmpstr wants to merge 1 commit into
purescript-contrib:masterfrom
vmpstr:makewatt

Conversation

@vmpstr

@vmpstr vmpstr commented Jan 8, 2021

Copy link
Copy Markdown

The text in the exercise states that we should use MakeWatt as the data constructor. However, the test and the solutions use Watt. This PR updates the test and solutions to instead use MakeWatt as the text suggests.

@milesfrain

Copy link
Copy Markdown
Member

Thanks for finding this discrepancy and creating a PR to fix it.
The MakeWatt instruction was accidentally leftover from an earlier revision of #273. We forgot to update the instructions too.
The better fix is to remove that bit of instruction from the exercise text so it will read:

(Easy) Use newtype to define a Watt wrapper for Number, then define a calculateWattage function...

So removed "with MakeWatt as the data constructor" and also changed "unit" to "wrapper", since I don't want there to be any confusion with the unit value.

Let me know if you'd like to make that alternate PR.

@vmpstr

vmpstr commented Jan 8, 2021

Copy link
Copy Markdown
Author

While doing the exercises, I liked the MakeWatt part, since it made me think about what's the type and what's the data constructor, since they were named differently. But I'll defer to your judgement. I'm happy to make the PR to fix the wording. (I'll make that a bit later in the day).

Thanks!

@vmpstr vmpstr closed this Jan 8, 2021
@milesfrain

Copy link
Copy Markdown
Member

I liked the MakeWatt part, since it made me think about what's the type and what's the data constructor, since they were named differently.

That's a good point, and the reason why we added that MakeColumb example. But it's more idomatic to just use the same name.

If you think it would be helpful for readers, feel free to include any of the following in your PR:

  • Add another exercise following the Make* pattern for contrast. Maybe feature this before the Watt exercise. Could do something like: calculateEnergy :: Coulomb -> Volt -> Joule with MakeJoule.
  • Note in the text (and/or the exercise) that it's more idomatic to just use the same names for the constructors.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants