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

Improve Concept Exercise: Numbers (Freelancer Rates) #1049

Closed
9 of 10 tasks
junedev opened this issue Mar 18, 2021 · 28 comments
Closed
9 of 10 tasks

Improve Concept Exercise: Numbers (Freelancer Rates) #1049

junedev opened this issue Mar 18, 2021 · 28 comments
Assignees
Labels
x:action/improve Improve existing functionality/content x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/concept Work on Concepts x:module/concept-exercise Work on Concept Exercises x:rep/large Large amount of reputation x:size/medium Medium amount of work x:status/claimed Someone is working on this issue x:type/content Work on content (e.g. exercises, concepts)

Comments

@junedev
Copy link
Member

junedev commented Mar 18, 2021

Getting started

Here you can read about what Concept Exercises are and how they are structured:

If you have not done so yet, it is probably also helpful to do a couple of "Learning Exercises" (this is how they are called on the site) yourself.

Goal

The goal of this issue is to extend the existing concept exercise Freelancer Rates that teaches the numbers concept.

The exercise is already in good shape but currently only explicitly introduces numbers as a concept. Additionally it should also introduce arithmetic operators incl. shorthand assignment operators.

See this list for details on the learning curve we are aiming for.

Additional Learning Objectives

  • How to do perform the following 6 arithmetic operations: addition, subtraction, multiplication, division, remainder, exponentiation
  • What are shorthand assignment operators for those 6 operations and how to use them

(To avoid too many tiny concepts, both of these will be combined in one additional concept called arithmetic-operators.)

ToDos

Add Concept

  • Create a new folder arithmetic-operators in concepts and add the concept to the general config.json file (incl. adding it to concepts for the freelancer-rates exercise there)
  • Write the introduction.md, about.md and links.json file for the concept

Adjust the Concept Exercise

  • Add the introduction.md of the concept at the end of the introduction.md file of the exercise, make some adjustments in the content if necessary.
  • Adjust the design.md file to incorporate the new learning objectives.
  • Update the tasks in the exercise to include at least one use of one of the more unusual operators (% or **) and one sensible use of a shorthand assignment operator. You can add new additional tasks but you can also consider replacing/adjusting an existing task that might currently not teach much more than the other tasks already do. Updating the tasks includes the following files
    • instructions.md
    • hints.md
    • exemplar.js
    • stub file (freelancer-rates.js)
    • test file (freelancer-rates.spec.js)

Fixes for the Existing Content

This part is optional polishing "while you are at it" but can also be tackled in another issue/PR later.

  • Fix broken link in "Built-in Object" section
  • About.md for numbers could benefit from the following additions:
    • explain the 3 special numbers: NaN and +/- Infinity
    • show how to write numbers with underscore or exponential notation
  • Remove the section on comparison, it is now included in the comparison concept, instead just keep the link ("How to compare numbers is explained in ...")
  • The current exercise requires rounding to solve it. The introduction.md of the exercise currently does not include this. It should point the students to a list of the rounding methods, e.g. like this one, so they don't get lost reading about rounding in JavaScript. Also update the introduction.md of the numbers concept to keep them roughly in sync.
  • See comments below for other issues that students found that should be addressed.

Ideas for About.md of the New Concept

  • / does not perform integer division like in some other languages
  • behavior of % if one operand is negative, see MDN docs below the demo box
  • typical use of % for keeping a value inside a range/ "if you reach the end while iterating, start from the beginning" (not sure how to describe this well but explanation should also be understandable for someone who does not know ring theory)
  • ** does the same as the older Math.pow()

Help

You can choose to do this solo-style, or collaborate with multiple people on this. The suggested approach is to

  1. First accept this issue by saying "I'd like to work on this" (no need to wait for a response, just go with it) and optionally request that someone works with you (and wait for a second person to accept your request).
  2. Use this issue to discuss any questions you have, what should be included in the content and what not and to collect reference material.
  3. Create a PR and set "exercism/javascript" as reviewers. Additionally you can write in #maintaining-javascript that your PR is ready for review. Once you incorporated any critical feedback that the reviewer might give you and the PR is approved, it will be merged by a maintainer.

Credit

This contribution grants you an author slot. Add your username to the authors key in the config file.

@junedev junedev added enhancement 🦄 Changing current behaviour, enhancing what's already there good first issue help wanted and removed good first issue labels Mar 18, 2021
@junedev junedev added x:action/improve Improve existing functionality/content x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/concept Work on Concepts x:module/concept-exercise Work on Concept Exercises x:size/medium Medium amount of work x:type/content Work on content (e.g. exercises, concepts) labels Aug 2, 2021
@junedev junedev changed the title [V3] Improve Concept Exercise: Numbers (Freelancer Rates) Improve Concept Exercise: Numbers (Freelancer Rates) Sep 3, 2021
@junedev
Copy link
Member Author

junedev commented Sep 3, 2021

@NobbZ did this exercise and noted two things.

  • There is a applyDiscount stub provided which you don't really need to use. If you don't use it, it looks like there is a throw new Error(...) left even though everything is fine and all tests pass. Since "use helper functions" is not the main focus for this early exercise, it is probably best to remove that stub.
  • Task 2 states to round up the result but this is easily overlooked. Maybe make that part a separate sentence so it is more prominent.

@benallan
Copy link

benallan commented Sep 4, 2021

Hi, I just did this exercise and found one part a bit unclear:

If the freelancer bills the project manager per month, there is a discount applied

I assumed this meant that each full month gets a discount, and any leftover days are billed at the full day rate. But from the expected results in the tests it seems that the discount needs to be applied to all days regardless of whether they make up a full month.

(Apologies if this was clear to everyone else!)

@junedev
Copy link
Member Author

junedev commented Sep 4, 2021

@benallan Good point! We should clarify that in the instructions.

@junedev
Copy link
Member Author

junedev commented Sep 5, 2021

Another comment by a student who was confused:

Looking at all the community solutions, I've seen the Math.ceil() function applied directly to the applyDiscount() function. However, this function is being used by both the monthRate() and the daysInBudget() functions. Additionally, it seems like the instructions wants me to treat both values from the aforementioned functions differently.

@junedev junedev removed the enhancement 🦄 Changing current behaviour, enhancing what's already there label Sep 11, 2021
@JaPrad
Copy link
Contributor

JaPrad commented Oct 3, 2021

I'd like to work on this

@junedev junedev added the x:status/claimed Someone is working on this issue label Oct 3, 2021
@junedev
Copy link
Member Author

junedev commented Oct 3, 2021

@JaPatGitHub Thanks a lot for picking this up! There are a lot of todos here. Feel free to only tackle some of them first. Your PR does not have to include all things at once. Also if you don't feel like doing all the parts we can put the rest in some new issue later.

@JaPrad
Copy link
Contributor

JaPrad commented Oct 3, 2021

Hello @junedev, my pleasure to work on this!
Where do I find the About.md file you mentioned under Fixes for Existing Content of this issue?
Did you mean introduction.md?

@junedev
Copy link
Member Author

junedev commented Oct 3, 2021

Please make sure you have read the documentation listed under "Getting started" above. There you learn that there are the concepts (in this numbers) and concept exercises (freelancer-rates), they exist in different root level folders. About.md is part of the numbers concept: https://github.com/exercism/javascript/blob/main/concepts/numbers/about.md

@JaPrad
Copy link
Contributor

JaPrad commented Oct 4, 2021

What do I do for the uuid of arithmetic-operators in the general config.json file?

@junedev
Copy link
Member Author

junedev commented Oct 4, 2021

You can use configlet to generate the uuid: https://github.com/exercism/configlet#configlet-uuid

How to fetch configlet is documented here: https://github.com/exercism/javascript/blob/main/CONTRIBUTING.md#fetch-configlet

@JaPrad
Copy link
Contributor

JaPrad commented Oct 4, 2021

Thank you @junedev. Went through the links you sent and tried implementing. I don't exactly get how to run the fetch-configlet binary on my windows computer (I've downloaded my fork of the repository locally). How do I do that?

Not much great help on google searches

@SleeplessByte
Copy link
Member

@JaPatGitHub if you're on windows, open PowerShell and cd to the javascript folder. Then run https://github.com/exercism/javascript/blob/main/bin/fetch-configlet.ps1 instead like this:

bin/fetch-configlet.ps1

If you're not on windows, cd to the javascript folder, then run https://github.com/exercism/javascript/blob/main/bin/fetch-configlet like this:

bin/fetch-configlet

Both will result in configlet or configlet.exe to be placed in that bin folder.

@Fbrufino
Copy link

Fbrufino commented Oct 9, 2021

Hi, I just did this exercise and found one part a bit unclear:

If the freelancer bills the project manager per month, there is a discount applied

I assumed this meant that each full month gets a discount, and any leftover days are billed at the full day rate. But from the expected results in the tests it seems that the discount needs to be applied to all days regardless of whether they make up a full month.

(Apologies if this was clear to everyone else!)

That wasn't my exact thought process, but I also was confused at task 3 as to which rate I was supposed to use, and even though I could guess from the function inputs and also the clue that

"If the freelancer bills the project manager per month, there is a discount applied. This can be handy if the project manager has a fixed budget",

I thought it was more vague than it should. In the end, it seems the whole monthly rate thing could actually be dispensed with in favor of talking directly in terms of something like a single-charge rate or a fixed-budget rate. That would make things clearer.

Also, I've just checked the hint for this task before commenting here, and I think it can even add to the confusion by mentioning dayRate, i.e., the other function. It seems to be suggesting the user to use that instead of monthRate. I think it can be improved just by making the following change:

old new
"First determine the dayRate, given the discount" "First determine the day rate given the discount"

@JaPrad
Copy link
Contributor

JaPrad commented Oct 15, 2021

@Fbrufino, I agree with you. I got your second point about changing dayRate to 'day rate' in the hint.
But I don't exactly get your first point - why do you think it's vague? What changes do you recommend?

@Fbrufino
Copy link

Fbrufino commented Oct 15, 2021

@Fbrufino, I agree with you. I got your second point about changing dayRate to 'day rate' in the hint. But I don't exactly get your first point - why do you think it's vague? What changes do you recommend?

Ok, I'm trying my best not to waste other people's time, so I tried to reanalyze both the instructions and my thought process when doing the exercise. I'm laying my conclusions here as clearly as I can so any mistakes can be easily pointed out.

I think there are 2 ways of interpreting task 3:

  1. In isolation: the discount passed as an input is and ad-hoc one which was already agreed upon between the freelancer and their client and thus should just be applied. The phrase in bold "If the freelancer bills the project manager per month, there is a discount applied. This can be handy if the project manager has a fixed budget" refers only to the possibility of applying a discount, not the whole possibility of billing monthly and then applying a discount. Finally, the proper hint for the task was just describing in detail how you should go about the calculation process;

  2. In the context of tasks 1 and 2: the discount passed as an input is the same "monthly billing" discount used in task 2 and the phrase "This can be handy if the project manager has a fixed budget" was meant to hint, albeit vaguely, at the connection between the monthly rate and what's used when there's a fixed budget. Finally, the proper hint for the task was a slight mistake for the usage of dayRate, the name of the function in the stub for task 1, and it further confirms that the author was thinking in terms of one of the functions for task 1 or 2. In the end, there's also the question of how exactly to fit/apply the monthly discount in this more general case, there's the aforementioned vagueness.

Now, granted, interpretation 1 is pretty clearly the more solid one between the two, but if you start out at interpretation 2, as I did, I think it's a bit hard to step out of it, it's just easier to try things out and see what the tests return. I'm almost definitely biased towards justifying my interpretation but the fact that the only explicit/clear mention of a discount is in the case of monthly billing might "prime" people towards the same reading I made. I suspect this accounts pretty well for @benallan's thought process too.

As to a solution, I think we can either try to do something to put task 3 more clearly out of the context of tasks 1 and 2; or try to more clearly lay out the situation in task 3 as a possibility in the "story" part, so as to avoid the "priming" effect I alluded to. Or both, of course.

My pitch:

  • REPLACE "If the freelancer bills the project manager per month, there is a discount applied. This can be handy if the project manager has a fixed budget." FOR "There are some situations where a discount is agreed upon, such as when the freelancer bills the project manager per month, or when there's a fixed budget for a project.";
  • MAKE TASK 3 THE SECOND TASK. Correspondingly, CHANGE THE ORDER OF THE FUNCTIONS in the stub to dayRate, applyDiscount, daysInBudget, monthRate. In this new ordering, the last task would arguably be easier than the second, but I think it would be an acceptable compromise;
  • In the description for task 3 (now task 2), REPLACE "Calculate the number of workdays given a budget, rate and discount" FOR "Calculate the number of workdays given a fixed budget, an hourly rate and a discount"

@junedev
Copy link
Member Author

junedev commented Oct 15, 2021

I agree with @Fbrufino but I would even go further than the changes he suggested

  • Use task 3 as task 2 and completely remove the discount from that task. It is fine if that task is just about doing division and using Math.floor.
  • As for the descriptions, I would recommend removing this part from the "introduction" section entirely.
    We first establish a few rules between the freelancer and the project manager:
    
    - The daily rate is 8 times the hourly rate;
    - A month has 22 billable days.
    
    If the freelancer bills the project manager per month or more, there is a discount applied. This can be handy if the project manager has a fixed budget.
    
    Discounts are modeled as fractional numbers (percentage) between `0.0` (`0%`, no discount) and `0.90` (`90%`, maximum discount).
    
    Instead present that information in the task where it is needed.
    So each task has a short "story" part and then the Implement ... section. For example
    ## 1. Calculate the day rate given an hourly rate
    The freelancer contract defines that the daily rate is 8 times the hourly rate.
    
    Implement ...
    

With these two changes there is no confusion anymore which information needs to be applied in which task. applyDiscount is not reused (and should be removed in any case) so there is also no confusion whether to apply the rounding there or not. Only the new task 3 would mention the discount.

@JaPatGitHub Do you think you can work with that input?

@JaPrad
Copy link
Contributor

JaPrad commented Oct 17, 2021

Thank you for your detailed suggestion @Fbrufino!
@junedev, I can work with this.

Only would like to know - are there any docs or instructions for writing test files?
If not, I believe @SleeplessByte can help me if I get stuck writing it?

@JaPrad
Copy link
Contributor

JaPrad commented Oct 17, 2021

Moreover, I have some thoughts about introducing % operator into the exercise:
Modify the exercise the way @benallan mistook it first to be. The 3rd task (currently 2nd) would be applying the discount for whole months and the leftover days are billed fully.

How do you all feel about this?

@junedev
Copy link
Member Author

junedev commented Oct 17, 2021

@JaPatGitHub

Regarding the test files
I can also help you with that. Tbh I don't think we have written down the latest rules for the test files that we established over the past months. I would recommend you look at an example, e.g. https://github.com/exercism/javascript/blob/main/exercises/concept/mixed-juices/mixed-juices.spec.js
Here an outline of the structure you can see there.

  • the tests are based on jest
  • each function has a describe block (there is no need for a describe block that wraps all the other describe blocks)
  • each test case is written with test function, the name of the test case should be descriptive (e.g. 'calculates the number of limes needed to reach the target supply' instead of limesToCut(42, ...) or with 42)
  • one test case can contain multiple expect statements if needed
  • all code needed for the test case needs to be written inside the test function, otherwise it will not show up on the website test runner output (in the case of this exercise, it is ok to have DIFFERENCE_PRECISION_IN_DIGITS as a constant outside though)
  • we currently do NOT use table driven test (test.each, forEach, etc.) because they also don't render well on the website
  • all tests are un-skipped in concept exercises (so test instead of xtest)

Let me know if you have further questions. (I will create an issue for transferring this info to some documenation file.)

Regarding the remainder operator task
I like the general idea you proposed regarding the month. However, if we bring back the discount into the new task 2 we get a lot of the issues back we were trying to avoid. Instead I would recommend adding a new task that could go like this:


Some clients always extend the contract by another full month. When the freelancer already worked for that client for a larger number of days, it would be good to know how many days are left to complete the current month so the freelancer knows it might be time to discuss another extension of the contract.
Each month has 22 billable days. Note than the number of days the freelancer already worked can span multiple months, see example below.

Implement a function daysLeftToFullMonth that takes a number of days the freelancer already worked for that client as an argument. It returns the number of days the freelancer still needs to work for that client to complete current month.

daysLeftToFullMonth(64)
// => 2
// 2 full month worked already, 20 days worked in third month -> 2 days left to complete third month

I know the story is not as nice as in your suggestion but it would avoid convoluting one task with too many aspects of the concept at the same time.

@JaPrad
Copy link
Contributor

JaPrad commented Oct 20, 2021

@junedev
Regarding Test Files
Thank you! I find these instructions comprehensive enough to start working. Will mention here if I face any difficulties :)

Regarding Remainder Operator
I think we had a miscommunication - my idea was meant for the new task 3 (not 2 - it stays clear of discount calculation)

However, I find your story good too. So which one do you think should we work on?

@junedev
Copy link
Member Author

junedev commented Oct 20, 2021

Sorry for the misunderstanding. I think a new task to implement something like priceWithMonthlyDiscount is better than my suggestion. You just need to make sure that it is clear that in the daysInBudget task the discount applies to all the days. You can do that on the story level, e.g. "If the number of days in the freelancer contractfffspans multiple month, there can be a certain discount that is applied for each full month. ..." vs. for the last task "Sometimes freelancer and client agree upon a fixed budget in the contract. Then each day that was worked uses up the budget at a discounted rate. ..."

@JaPrad
Copy link
Contributor

JaPrad commented Nov 8, 2021

@junedev, I wish to add everyone who gave suggestions in this issue in the .meta/config.json contributor list.
Are you okay with that?

@junedev
Copy link
Member Author

junedev commented Nov 8, 2021

@JaPatGitHub That is very nice of you but the contributor slot is meant for the person that actually did the change in the code and that means you. 🙂

@JaPrad
Copy link
Contributor

JaPrad commented Nov 10, 2021

I've been thinking about the use of shorthand arithmetic operators in the exercise. Do you think we can make sensible use of it in this exercise? In my experience, I've mainly used shorthand operators only in loops - beyond the scope of this exercise.

@junedev
Copy link
Member Author

junedev commented Nov 10, 2021

@JaPatGitHub Good point, I asked myself the same. I couldn't think of a good exercise here and there are a couple of other exercises later where these operators are used and the analyzer can tell students if they didn't use them there. So that's good enough for now.

@junedev
Copy link
Member Author

junedev commented Nov 10, 2021

@JaPatGitHub All the main points from this issue are addressed now. 🎉 Thanks again! The only thing left are some improvements for the about.md file. Do you want to work on those as well? If not, I would move them to a new issue and consider this one closed.

@JaPrad
Copy link
Contributor

JaPrad commented Nov 11, 2021

@junedev, I'm glad to have worked on this. This was my first open-source contribution apart from fixing typos and broken links. A personal thank you for your reviews - made me go a long way!

I plan to stay dormant on GitHub for the rest of this month as I have my exams and the courseload is heavy. Hence I think it would be better to move to a new issue so that someone else can work on it or I tackle it later :-)

@junedev
Copy link
Member Author

junedev commented Nov 11, 2021

The ticket with the leftover items is here: #1505

@junedev junedev closed this as completed Nov 11, 2021
@junedev junedev added the x:rep/large Large amount of reputation label Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
x:action/improve Improve existing functionality/content x:knowledge/intermediate Quite a bit of Exercism knowledge required x:module/concept Work on Concepts x:module/concept-exercise Work on Concept Exercises x:rep/large Large amount of reputation x:size/medium Medium amount of work x:status/claimed Someone is working on this issue x:type/content Work on content (e.g. exercises, concepts)
Projects
None yet
Development

No branches or pull requests

5 participants