Hi everyone,
I recently completed the Lasagna Master exercise in the JavaScript track. During a discussion with mentor Safwan (@safwansamsudeen), we discussed whether the portions parameter is considered optional or required. If considered optional, that scenario needs to be handled differently to prevent the factor from return NaN. The instructions seem to suggest that portions is optional (I think), but should portions be considered a required parameter?
What track is this for?
JavaScript.
(I was the mentor) I’m not too sure if this is even unintentional - but I do feel we could clarify things. To give more detail, the exercise instructions, task 4, do not indicate that second parameter portions could be optional, but a test checks:
test('works for an empty recipe', () => {
expect(scaleRecipe({})).toEqual({});
});
We either modify the instructions to reflect this, or modify the test to have some random portions value, which would be a better change as making portions optional would require other tests that actually test that functionality (this one doesn’t, as an empty recipe causes the for loop to be skipped).
@amitchell05 could you move this to the JS category?
@safwansamsudeen Thank you for joining the discussion, Safwan! I’m new to this forum and wasn’t sure if I could tag by name. I couldn’t find the JS category or tag. For categories, I only see Exercism, Announcements, Programming, Exercism Support, and Social. For tags that are language-specific, I only see python.
Update: I found it! JavaScript was under Programming. I’m sorry for the confusion on my end.
@IsaacG Yes, as Safwan mentioned, it’s for the JavaScript track. I’m sorry for not including that in my initial post. I’ve updated it now.
I think the idea here is that an implementation as follows doesn’t “care” about the second parameter being Not A Number:
export function scaleRecipe(recipe, portions) {
const scaled = { ...recipe }
const factor = portions / 2
for (let ingredient in scaled) {
scaled[ingredient] *= factor
}
return scaled
}
When the recipe is empty, the factor is never applied.
That said, I don’t think there is a very good way to determine a default value that is a number. When the recipe is empty, there is nothing to scale. The answer for 1 and for 1000 is the same ({}). So I would pose the question back to both you @amitchell05 and @safwansamsudeen:
What do you feel would be a good default value?
If we can determine a good default value then the options are:
However, I don’t think there is a good default value. Scaling doesn’t make sense for 2, (or 0) portions, and introducing 0 as a default or 1 or 2 as a default doesn’t feel right. In the case of 0 we’d expect a shortcut that returns {} always. In the case of 2 case we would expect a no-op that returns the input. In the case of 1 we can do the algorithm, but why 1. Why would having no portions mean I want to reduce it to half / one portion?
I think the by far easiest way to deal with this is to add a line in the instructions that says: If there are no ingredients at all, the second parameter portions should be ignored.
In that case, people may start to write:
export function scaleRecipe(recipe, portions) {
if (Object.keys(recipe).length === 0) {
return {} // or return recipe
}
const scaled = { ...recipe }
const factor = portions / 2
for (let ingredient in scaled) {
scaled[ingredient] *= factor
}
return scaled
}
I don’t mind that. It’s not bad JavaScript or non idiomatic.
I’m a bit confused and probably missing something important. The instructions read:
“A recipe object that holds the amounts needed for 2 portions”
This suggests that the portions parameter could be made optional with a default value of 2. So if portions is omitted, the tests still pass, the factor equals 1, and there’s no contradiction with the tests. Since there a test case that ignores the portions parameter, this looks like an indication that portions is considered optional.
That seems good to me. I originally thought we can add an instruction that says if portions are undefined, the student returns the recipe immediately - but on thought, I don’t like it much as having a scale function and not passing in the parameter doesn’t make much sense.
Could we also change the aforementioned test to pass in some value of portions?
a) why would the portions parameter be omitted?, b) if it is meant to be omitted, it should say so more explicitly in the test.
True. Which is why I think we should also change the test case.
I’m not sure if i understand the question. If it is optional it can be omitted.
I don’t believe the tests are the right place for this. If a parameter is optional this should be mentioned in the instructions. Tests are for testing, not for giving instructions.
Also, how did the empty recipe came into play? How does it affect whether a parameter is optional? I still don’t follow the logic.
Ah I’m sorry I meant to say " if it is meant to be omitted, it should say so more explicitly in the instructions."
You’re quite right!
Not providing a factor for scaling the recipe seems to defeat the idea behind scaling a recipe to me. My point was that it shouldn’t be optional - but if it is decided to be so, then we should say so in the instructions, as you said.
I agree with @SleeplessByte that there exists no sensible default parameter… because it shouldn’t be omitted at all in the first place.
I agree it’s pointless, but this does not seem to cause any issues. If we assume that the factor is 1 when the portions is omitted (so default value of portions is 2), then the recipe simply does not scale. The instructions do not suggest that the function should always scale the recipe. That’s because if the user calls the function with portions = 2 then the recipe isn’t going to be scaled anyway. So i believe there’s a case for making portions optional.
But it looks as if the simplest solution is to use a random value for portions in that test.
Also, let’s not forget that the user is already free to make portions optional if they want. They instructions do not need to mention this.
I apologize for the extremely late reply to my post. I am curious if defaulting the portions value to 2 would be a good idea or not. For one of my iterations, I set it that a way and had the scaled object remain empty. It gets populated only if recipe is not empty:
export function scaleRecipe(recipe, portions = 2) {
const factor = portions / 2;
const scaledRecipe = {};
for (let ingredient in recipe) {
scaledRecipe[ingredient] = recipe[ingredient] * factor;
}
return scaledRecipe;
}
However, I do see the benefit of only mentioning in the instructions that the portions should be ignored and that the function should end early if it is empty.