Basics-2 concept and associated exercise #329
Conversation
|
I'll make one concept exercise today to remember how that works and give a review to this afterward |
There was a problem hiding this comment.
Thanks for doing that fundamental concept! I've taken some time to review it and I like the string formatting idea! There are things that need to change however in my opinion.
Let start with the concept name. It feels weird to have a "fundamental" concept after a "basics" concept. To me, and maybe that's because of my French background, there is no clear cut of what should come first between the two. I was wondering if we could do something like basics-follow-up or something else that would make this clearer that it's directly following basics and not before or parallel. Let me know what you think on that naming details!
Regarding the global shape of the PR, I've made some updates to fit with the current spec. In particular, I've readded the basics/introduction.md file which is needed (and will be needed here too) and changed the titles and levels of the different markdown documents to fit #330.
Then regarding the concept, there are 3 files needed. Chronologically, student will face the concept introduction.md file first, then the concept exercise introduction.md file, and finally the concept about.md file. What I like to do is starting with the concept exercise introduction.md since it's the thing directly related to the exercise, then expand it in the about.md where I can add some information that wasn't totally relevant for the exercise or giving too much detail but are important as a summary of what was learn. Finally I shrink it down in the concept introduction.md to have just the minimal information needed to understand what this concept is about and what will be learned. So I've started to review the concept exercise introduction.md.
I've done some changes to it, let me know what you think and don't hesitate to change it again.
I've first switched the exporting and importing sections. Exporting introduces the concept of exposing and was seen in the previous basics concept so I thought it would make more sense to have it first. Then I've made adjustments to the text of the importing and string sections. They have their own commits so you can easily see the changes.
Finally I've had a look at the exercise instructions. I like the string formatting idea! However we should aim for easy incremental tasks in concept exercises, so I think we should split the formatPrice function in steps. (1) a task to compute the amount of pounds: 599 -> 5. (2) another task to compute the left pennies: 599 -> 99. (3) finally a task to generate the formatted price. By the way, I think the conversion to floats is asking for trouble ^^, we might end up with non-round values. So I'm not sure if we should have floats in the learning objectives.
|
Hi Matthieu, I'm happy with your changes.
I must have done this for a reason, although I forget what it is now. I imagine I was working from an example fshare concept, or from some out of date documentation. You seem to be much more on top of all the ongoing changes to the specification than me. Are you just spending a lot of time on slack / github and working it all out, or is there a reference document somewhere that always contains the latest spec?
Fair enough. I think there was initially only one of these, but then we added a "minimal compilable file" concept, that we called basics. I have no strong feelings about the name, its mostly just to simplify the concept dependency tree, somaybe basics-2? basics-follow-up doesn't sound quite right to me, but I don't think its a big deal, and I wouldn't object if you prefer it.
I'm not sure about this. It does make the tasks more incremental, but it does also make the solution more complicated overall (getting 99 from 599 requires more work than just dividing 599 by 100). I'm sure you could persuade me though.
We probably will end up with numbers that can't be represented properly at two decimal points, but I don't think it will be a big issue. The same thing should happen in the test that happens in the code, so the tests will still pass (although it might look disconcerting for a student when looking at test failures or debug logs). Your plan of calculating the integer number of pence does sort this out, if we go with it. Alternatively we could add something to the description perhaps? Or choose numbers for the test that we know do work at two decimal points. I think either this or the basics concept (or a new concept to go after this one) does need to introduce So I think the options are:
We could also make the tasks incremental, but in a way that doesn't increase the complexity. Maybe just by converting 599 to 5.99 as one step, and then by formatting as a string as another step. This still means that we don't worry about the floating point conversion. |
|
Are you just spending a lot of time on slack / github
You could say that ^^. I've been following things more closely lately.
is there a reference document somewhere that always contains the latest
spec?
More or less, that's why I want to do that one "how to" document to make a
concept + exercise with links and context. Generally you have up-to-date
docs in that folder:
https://github.com/exercism/docs/tree/main/anatomy/tracks
…On Wed, Feb 17, 2021 at 11:55 AM Cedd Burge ***@***.***> wrote:
Hi Matthieu, I'm happy with your changes.
Regarding the global shape of the PR, I've made some updates to fit with
the current spec. In particular, I've readded the basics/introduction.md
I must have done this for a reason, although I forget what it is now. I
imagine I was working from an example fshare concept, or from some out of
date documentation. You seem to be much more on top of all the ongoing
changes to the specification than me. Are you just spending a lot of time
on slack / github and working it all out, or is there a reference document
somewhere that always contains the latest spec?
Let start with the concept name. It feels weird to have a "fundamental"
concept after a "basics" concept.
Fair enough. I think there was initially only one of these, but then we
added a "minimal compilable file" concept, that we called basics. I have no
strong feelings about the name, its mostly just to simplify the concept
dependency tree, somaybe basics-2? basics-follow-up doesn't sound quite
right to me, but I don't think its a big deal, and I wouldn't object if you
prefer it.
However we should aim for easy incremental tasks in concept exercises, so
I think we should split the formatPrice function in steps. (1) a task to
compute the amount of pounds: 599 -> 5. (2) another task to compute the
left pennies: 599 -> 99. (3) finally a task to generate the formatted price.
I'm not sure about this. It does make the tasks more incremental, but it
does also make the solution more complicated overall (getting 99 from 599
requires more work than just dividing 599 by 100). I'm sure you could
persuade me though.
By the way, I think the conversion to floats is asking for trouble ^^, we
might end up with non-round values.
We probably will end up with numbers that can't be represented properly at
two decimal points, but I don't think it will be a big issue. The same
thing should happen in the test that happens in the code, so the tests will
still pass (although it might look disconcerting for a student when looking
at test failures or debug logs). Your plan of calculating the integer
number of pence does sort this out, if we go with it. Alternatively we
could add something to the description perhaps? Or choose numbers for the
test that we know do work at two decimal points. I think either this or the
basics concept (or a new concept to go after this one) does need to
introduce float.
So I think the options are:
- Leave it as it is, and not worry that float conversion may require
more than 2 decimal points to represent a number (and potentially add a
mitigation, such as only testing numbers that can be represented).
- Make the tasks incremental as you suggest, introduce a new float
concept and exercise, and not worry about the extra complexity.
We could also make the tasks incremental, but in a way that doesn't
increase the complexity. Maybe just by converting 599 to 5.99 as one step,
and then by formatting as a string as another step. This still means that
we don't worry about the floating point conversion.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#329 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWFOCJAX6GSHTWM6EXUPTTS7OOD3ANCNFSM4XJUBPFQ>
.
|
Thats good to know. The documentation there is pretty thin, but if its thin and up to date thats still a lot better than what I have at the moment! Have you had any thoughts about the rest of it? |
I'm good with that, I think that's better than fundamentals. We should rename basics to basics-1 then.
Well, getting 5 out of 599 is just
I'd be fine with it too. It depends if you want to keep floats in basics-2. One thing that isn't optimal with this exercise is that concept exercise should mostly have 1 good solution, contrary to practice exercises, and as we can see, there are at least 2 or 3 valid ones here so this makes me doubt actually. But if we do this:
the types of the function tasks may be sufficiently constraining so that there is only one good way of doing it. So that's probably the right choice. |
You are an unusually good programmer Matthieu! I think if you haven't come across this problem before, and are not confident with maths, this can easily take a while to work out. |
|
So, this is what I think we have agreed as the way forward. Please let me know if I've understood correctly, and then I'll make the changes.
|
|
Yep ! For the tests, I'm not sure what you mean by "range" I was thinking a
few unit tests with values with verified give good errors should be fine
…On Sun, Feb 21, 2021, 11:35 Cedd Burge ***@***.***> wrote:
So, this is what I think we have agreed as the way forward. Please let me
know if I've understood correctly, and then I'll make the changes.
- change basics to basics-1
- change foundations to basics-2
- Change exercise and tests, to convert 599 to 5.99 as one function,
and to convert 5.99 to "£5.99" as another function.
- Remove the fuzz tests, and instead using a range of numbers that we
know can be represented successfully at two decimal places after being
divided by 100 and don't cause any other complications.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#329 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAWFOCPHRS5F6BPLP6EGMDLTADOWBANCNFSM4XJUBPFQ>
.
|
|
Great, I'll get on to that.
This is what I meant! "range" probably wasn't the best word, maybe "selection" would have been better. |
ceddlyburge
changed the title
|
Hi Matthieu, I've updates this pull request as we discussed, can you give it another look and then I'll tidy up the conflicts and merge it in. |
|
awesome! I'll have a look at it during the week, sorry I was busy finishing up elm-test-rs this weekend |
Maybe I have misunderstood their respective purposes a bit
Exporting introduces the notion of exposing and was already introduced in the previous concept so putting it first seems more natural to me.
mpizenberg
force-pushed
the
foundations-concept
branch
from
96f6cfb to
97e032a
Compare
|
I've made some docs adjustments and spec-compliance changes. I think this is good enough to be merged. Both basic exercises are in a "wip" status but at least we'll be able to try them on exercism.lol. |
ErikSchierboom
added
x:size/large
This is a first pass at the foundations concept and associated exercise
instructions and about are near identical, but I think this is intended for some reason I don't understand yet
links and hints are also very similar, and I think this is probably more of an issue, as I don't think there is any real reason for them to be, so probably I haven't understood something at one of them might want to change
I haven't sorted out the configlet tools either, but will probably get that set up if people are happy with the overall direction of this PR