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

resistor-color-trio: add exercise #1551

Merged
merged 14 commits into from
Jul 24, 2019

Conversation

ErikSchierboom
Copy link
Member

Closes #1549

"input": {
"colors": ["orange", "orange", "black"]
},
"expected": "The value of this resistor is 33 Ohms."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it sufficient to just expect 33 Ohms? The rest of the sentence seems unnecessary.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like when programs talk in real sentences. So, just returning the value is too little imo. But I agree that the sentence is adding noise...

I see 2 options: either a shorter sentence, like "This is a 33 Ohms resistor". Or "The value of orange-orange-black is 33 Ohms".
That latter would introduce a second iteration on colors. I'm not sure if that adds something interesting.
@ErikSchierboom I don't mind going with the shortened sentence. Do you?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the problem specification, I'd prefer if the expected value were the integer 33 instead of any kind of string.

String formatting is not integral to this exercise, and a trivial wrapper function makes a sentence out of an integer. The reverse operation is less simple.

We should be teaching good practices. One good practice is to avoid conflating two different logical operations into one function. Computing a resistor's value from its colors and formatting a human-friendly string describing a resistor are two different logical operations.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the problem specification, I'd prefer if the expected value were the integer 33 instead of any kind of string.

I’d strongly second that point. The first two exercises are focused on calculating the integer value by parsing a string, and this adds on the complexity of that from the previous two. Adding string interpolation is unnecessary and it’s not like we’re lacking in exercises for that anyway. What we have been lacking is simple exercises that gradually expand on integer maths.

The very debate about preference about what string to return displays another problem with having one function that addresses two concerns; the string you’d print would be culturally and locale sensitive in the real world, so you’d want one function doing the math and another doing the interpolation. If we don’t want to add a second function, then should we make another exercise that spends time on English-specific string formatting esoterica (ie 3000 > 3K)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the concept of separating the two concerns, one calculating the values, and one for returning a string.
I do think we need the string part of this exercise. Not the interpolation, but for the part that is about Ohms vs kilo Ohms.
That is also meant as a bridge to the next Resistor exercise(s), where the Ohms value is combined with other values in other units (like a percentage for accuracy).
My proposal would be: separate the concerns in 2 methods, one for calculation, one for the ‘report’/printing. Can we all live with that?

Copy link
Contributor

@emcoding emcoding Jul 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we make another exercise that spends time on English-specific string formatting esoterica (ie 3000 > 3K)

Agreed. And IIRC there’s an issue to create such an exercise.

Copy link
Member

@coriolinus coriolinus Jul 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, a two-function solution works for me. One accepts as input a list of colors and returns an integer; the other accepts an integer and returns a string. We should test them independently.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the concept of separating the two concerns, one calculating the values, and one for returning a string.

Fine with me!

One accepts as input a list of colors and returns an integer; the other accepts an integer and returns a string.

What do you think @F3PiX?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm totally okay with it 👍

@emcoding emcoding added the Track Anatomy Project Related to or helpful to the Track Anatomy Project label Jul 15, 2019
@ErikSchierboom
Copy link
Member Author

I've tried to address the various comments made.

exercises/resistor-color-trio/description.md Outdated Show resolved Hide resolved
exercises/resistor-color-trio/description.md Outdated Show resolved Hide resolved
exercises/resistor-color-trio/description.md Outdated Show resolved Hide resolved
"input": {
"colors": ["blue", "grey", "brown"]
"colors": ["green", "brown", "orange"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/orange/brown to get 510

"description": "Brown and black and black",
"property": "description",
"input": {
"colors": ["brown", "black", "black"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The inputs for the description property should be integers, not color lists. The student has already written a three-color conversion function, at this point. No need to force them to call it; it is both obvious and natural to hook the output of the color converter into the input of the describer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except that the integers don't mean anything by themselves.

"orange" "orange" "orange" would be [3, 3, 3].

(10 * colorValue('orange') + colorValue('orange')) * (10 ** colorValue('orange'))

Would be a readable solution. If we use integers, we are moving away from correct and optimal solutions that look like this:

(colorFirstValue('orange') + colorSecondValue('orange')) * colorMultiplier('orange')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SleeplessByte can you expand on that objection? If I'm reading @coriolinus correctly he's suggesting that value("orange", "orange", "orange") should return 33000 and thus both description(33000) and description(value("orange", "orange", "orange")) should return the same formatted string ... ie description should take ohms as its argument, not band colors. Because value would already be returning ohms I'm not sure how that influences the solution as you've described it.

That said maybe we've got some people thinking:

def value(color, color, color):
    return ohms

def description(color, color, color):
    humanized_ohms = [some operations on value(color, color, color)]
    return "blah blah" + humanized_ohms

Is preferable while others are assuming:

def value(color, color, color):
    return ohms

def description(ohms):
    humanized_ohms = [some operations on ohms]
    return "blah blah" + humanized_ohms

Is the natural form.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't mean to suggest a list of integers; I meant a single integer, the value in ohms.

The intent is to get people to write one calculate_value function, and one format_description function. It should be obvious and trivial to nest the calls: if what you have is a list of colors, and what you want is a string description, there's no reason not to write format_description(calculate_value(color_list)).

However, it would teach bad factorization to include both operations in a single function.

Copy link
Member

@SleeplessByte SleeplessByte Jul 17, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah. I understand what you're (both, @coriolinus and @yawpitch) are saying. I think having value return 3300 is fine 👍 😄 .

I think having label return "33 kilo Ohms" is also something we should have.
I think having description is something a track can add based on preference on a sentence.

I don't agree with all the "this will lead to bad code" arguments. I understand what you're saying, but it's up to the student to recognise the two concerns or for the mentor to point them out. Purposefully crafting problem descriptions so a student can't do it wrongly is kinda forcing a certain implementation, which is something we don't want.

I personally think that forcing description to take "ohms" is also a language-specific implementation details. Having a class object with a description property that doesn't take arguments is very reasonable in a lot of languages. Both your arguments are forcing a certain code style (that is functional).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See: #1551 (comment)

value as number (33000) is still an incorrect abstraction.

Copy link
Contributor

@yawpitch yawpitch Jul 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I personally think that forcing description to take "ohms" is also a language-specific implementation details. Having a class object with a description property that doesn't take arguments is very reasonable in a lot of languages. Both your arguments are forcing a certain code style (that is functional).

I'm not sure that it does; it's up to the track implementer to decide on function vs class as the organizing structure, and for instance in Python I might reasonably decide that this should be the excercise in which we introduce subclassing of immutables and implement this as:

class ResistorValue(int):
    def __new__(cls, color, *colors):
         pass

    @property 
    def description(self):
        pass

Now description is perfectly valid and doesn't take ohms as a functional parameter, because the class itself is inherently a value in ohms.

@cmccandless note to us, might be a thing to consider for this exercise, as lord knows we don't have anything that introduces properly subclassing immutable builtins.

@emcoding
Copy link
Contributor

On second thought, I'm not happy with the new tests, and I'd vote for going with the original set of tests (with the colors as input and the full description as output).

My thinking is: This exercise is like a mini-app, where a user inputs a list of colors they are reading from a resistor, and they get back the description with the value. To me it makes sense that that's the only thing we test.
The rest of the design are 'implementation details', that we don't need to test.
(This is similar to the Luhn tests: input is a string, output is valid true or false. https://github.com/exercism/problem-specifications/blob/master/exercises/luhn/canonical-data.json)

I can imagine that a student puts all calculation in the description method, and that will make the tests pass. Now that we have green, it's up to the mentor to initiate a refactor, and point out that the code works but that it is not a good design.

The 'testing the public interface' argument is the main reason.
A second reason is the learning part of the exercise. If the tests force the student to follow an upfront design, they don't need to think about why we should seperate concerns. When they start with a suboptimal design, the mentor naming the problem and let them solve it is a better learning opportunity.

TL;DR
My proposal is to revert to the previous set of tests. Which is not questioning the best practice in the implementation to separate calculation the value from composing the desciption.
I briefly discussed this with @ErikSchierboom, so for anyone else: can you too live with reverting to the original tests, and if not, why?

@yawpitch
Copy link
Contributor

yawpitch commented Jul 17, 2019

My thinking is: This exercise is like a mini-app, where a user inputs a list of colors they are reading from a resistor, and they get back the description with the value. To me it makes sense that that's the only thing we test.

Isn't that fundamentally true of every exercise? In that every exercise could be seen as either a re-useable library function or a discrete app, and that we could simply de-refactor every exercise to return a long descriptive string in the same manner?

My argument against that would partially be that it forefronts bad habits, but mainly that it forces our users to be fluent in vernacular English idioms and to recreate a string that they may not understand and which is mostly a matter of personal preference to begin with. In the Python track, at least, we strive not to test specific error text, for instance, to avoid putting up another barrier to students who are ESL (English as a Second Language) ... I think that's a good thing to consider across all tracks.

Also it seems to me that the TDD approach we've taken treats every exercise as a library by default, since testing of app integration is usually not done with unit tests.

(This is similar to the Luhn tests: input is a string, output is valid true or false. https://github.com/exercism/problem-specifications/blob/master/exercises/luhn/canonical-data.json)

Yes, but luhn is returning unambiguous and actionable data, as a good library function should in order to build applications with it. A string that encodes multiple pieces of useable data with (let's face it) human-readable fluff isn't an actionable piece of data. A response like "33 ohms" or "33 kilohms" (or kiloohms or kilo-ohms) would be more actionable, but 33 or 3300 would be most actionable. Wouldn't a separate exercise to handle "humanising" -- more accurately "English colloquialising" -- be more instructive than, essentially, re-hashing the same problem in multiple smaller exercises?

I can imagine that a student puts all calculation in the description method, and that will make the tests pass. Now that we have green, it's up to the mentor to initiate a refactor, and point out that the code works but that it is not a good design.

Will that not end up being pointed out on most every submission? That just seems like overhead.

The 'testing the public interface' argument is the main reason.

Sure ... but why is the public interface as interpolated string necessary? What is that adding to the exercise across all tracks that are likely to implement the exercise? Don't we already have a significant number of exercises whose entire necessary interface is such a string? I'm open to the idea that it might be teaching something fundamentally new in relation to other exercises that do such string interpolation, I'm just not seeing what that is at the moment.

A second reason is the learning part of the exercise. If the tests force the student to follow an upfront design, they don't need to think about why we should seperate concerns. When they start with a suboptimal design, the mentor naming the problem and let them solve it is a better learning opportunity.

I agree there's a learning opportunity there, but I'm not sure it's better served by having the tests de-normalised. It's also an argument that can also be applied to all the exercises ... why not de-normalise others back to "bad" behaviour so students can be walked forward to "good"?

TL;DR
My proposal is to revert to the previous set of tests. Which is not questioning the best practice in the implementation to separate calculation the value from composing the desciption.
I briefly discussed this with @ErikSchierboom, so for anyone else: can you too live with reverting to the original tests, and if not, why?

The debate is mainly over what "value" is being returned. There seems to be a fair bit of controversy caused by that "value" being a sentence. That's certainly my issue, based on fielding a lot of issues with students who already struggle with English idioms and a large number of exercises that essentially rely on those idioms. Can I live with it too? Sure ... but I want to better understand the need for resistor-color-trio to deviate from resistor-color and resistor-color-duo, and from other exercises, by de-normalising the test specification towards an intentionally bad design. And I'd much rather we consider returning a value that the user can easily parse into an actionable form -- ie "VALUE UNITS" or better yet (VALUE, UNITS) -- rather than testing them, again, on string formatting to incorporate the actual data into an arbitrary series of words that may be semantically meaningless to them.

@coriolinus
Copy link
Member

Thank you @yawpitch for expressing exactly my objections with greater eloquence than I have available at the moment.

I think it is a very bad idea to intentionally denormalize the design: even if mentors will usually prompt students to normalize it, what fraction of the mentors will forget to do so? What fraction of students will simply move on to the next exercise, instead of continuing to refine an accepted solution, and thereby learn bad habits?

@SaschaMann
Copy link
Contributor

Will that not end up being pointed out on most every submission? That just seems like overhead.

Indeed, it seems like this would end up being an automated comment on the tracks that have auto-mentoring for common problems anyway.

I agree with @yawpitch but also want to add another objection to having only strings in the shared test data.

My thinking is: This exercise is like a mini-app, where a user inputs a list of colors they are reading from a resistor, and they get back the description with the value. To me it makes sense that that's the only thing we test.
The rest of the design are 'implementation details', that we don't need to test.

Testing a specific, arbitrary string representation of a value with a physical unit is also an implementation detail. You're forcing a specific way to implement string representation such values, that may go against the conventions in certain languages. E.g. in Julia, the canonical way to represent a resistance value would be 350Ω (note that this is not a string) with its string representation "350 Ω". Turning this into "350 ohm" would require reinventing the wheel and teach bad habits.

Of course, this is just how one language does it, but if this problem spec only specifies strings, it can't be used to create/generate tests without a lot of string-parsing and -converting hassle. When both the final string and the "pure-value" are part of the tests, this will be much easier because the strings can be ignored and instead be generated from the "pure-value" test data.

Copy link
Contributor

@yawpitch yawpitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g. in Julia, the canonical way to represent a resistance value would be 350Ω (note that this is not a string) with its string representation "350 Ω". Turning this into "350 ohm" would require reinventing the wheel and teach bad habits.

Sorry for the tangent, but note to self: learn Julia.

"description": "Brown and black and black",
"property": "description",
"input": {
"colors": ["brown", "black", "black"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SleeplessByte can you expand on that objection? If I'm reading @coriolinus correctly he's suggesting that value("orange", "orange", "orange") should return 33000 and thus both description(33000) and description(value("orange", "orange", "orange")) should return the same formatted string ... ie description should take ohms as its argument, not band colors. Because value would already be returning ohms I'm not sure how that influences the solution as you've described it.

That said maybe we've got some people thinking:

def value(color, color, color):
    return ohms

def description(color, color, color):
    humanized_ohms = [some operations on value(color, color, color)]
    return "blah blah" + humanized_ohms

Is preferable while others are assuming:

def value(color, color, color):
    return ohms

def description(ohms):
    humanized_ohms = [some operations on ohms]
    return "blah blah" + humanized_ohms

Is the natural form.

Copy link
Contributor

@emcoding emcoding left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a few suggestions, esp a different description, to solve the double 'the same way', and to explain what a kilometer or kilogram really is.

@SleeplessByte
Copy link
Member

SleeplessByte commented Jul 17, 2019

I've read all that is said above and whilst I mostly agree with the nits and content of the arguments by @yawpitch and @coriolinus , here are my two cents:

  • Prefer a single property in the canonical data. I suggest label which returns a formatted string (33 kilo Ohms). This exercise is meant to do 2 things: the algorithm math and the branching of humanisation. Adding extra text is not preferred (re: description), because in a real world program there are various use cases for label and very little for description. I agree with @coriolinus (and the second explanation by @yawpitch ) that description is composable.

  • Optionally allow tracks to add a local track HINT to implement value which returns a number which is the raw number of kilo ohms. The reason I don't think this should be in the tests is because it breaks the abstraction on the fourth band. I think the arguments here are all very alike primitive obsession. FYI, the fourth band may (depending on existence and colors of fifth / sixth) add an error margin e.g. 1%. value then giving 33000 is an incorrect abstraction. It should almost give a range, or a special object that has median 33000 and std of 0.01 * mu or whatever.

  • If languages / tracks choose to, add description which takes the label and turns it into a sentence.

re: @SaschaMann

Testing a specific, arbitrary string representation of a value with a physical unit is also an implementation detail. You're forcing a specific way to implement string representation such values, that may go against the conventions in certain languages. E.g. in Julia, the canonical way to represent a resistance value would be 350Ω (note that this is not a string) with its string representation "350 Ω". Turning this into "350 ohm" would require reinventing the wheel and teach bad habits.

This is super cool. Did not know and love that Julia has that! I think it still makes sense to have label have that, because it's that. A label printed on a box. However, I do think that if there were a value property, Julia should change the tests to test for 33000Ω and not 33000.

@SleeplessByte
Copy link
Member

SleeplessByte commented Jul 17, 2019

@F3PiX @ErikSchierboom I suggest adding the actual TABLE of coloured bands + indices + meaning to the description, instead of only the "numbers". The information here is incomplete. I also suggest doing that to all the exercises.

color significant figure / digit / value multiplier tolerance temp. coeff
- - - ±20 %
black 0 10^0 - 250 ppm / K
brown 1 10^1 ±1 % 100 ppm/K
red 🍓 2 10^2 ±2 % 50 ppm/K
orange 🍊 3 10^3 ±0.05 % 15 ppm/K
yellow 🍋 4 10^4 ±0.02 % 25 ppm/K
green 🍏 5 10^5 ±0.5 % 20 ppm/K
blue 6 10^6 ± 0.25 % 10 ppm/K
violet 7 10^7 ±0.1 % 5 ppm/K
grey 8 10^8 ±0.01 % 1 ppm/K
white 9 10^9 -
gold - 10^-1 ±5 %
silver - 10^-2 ±10 %
pink - 10^-3

We probably don't want to overwhelm the student with information, so cut from this table as you wish, but the notion that the INDEX of the band is not important (from the description) is probably an oversight.

🍓 does not mean 2
🍊 does not mean 3
🍋 does not mean 4
🍏 does not mean 5

🍏🍋🍊🍓 does not mean 5432 (but 54 kilo Ohms ±2% if I'm correct), however

🍋 in the first position means 4 * 10^1 (4)
🍋 in the second position means 4 * 10^0 (40)
🍋 in the third position means multiply by 10 ^ 4 (10 000)
🍋 in the fourth position means a tolerance of ±0.02% (unless they are super old, then yellow is gold is ±5% (???))

The position is significant. The descriptions of resistor-color, resistor-color-duo and resistor-color-trio do not make that apparent right now.

Erik Schierboom and others added 3 commits July 18, 2019 08:21
@yawpitch
Copy link
Contributor

yawpitch commented Jul 24, 2019

Taking inspiration from some suggested solutions in this thread, what if we encode both the label and the value in the expected output? Something like this:

{
    "description": "Orange and orange and black",
    "property": "label",
    "input": {
        "colors": ["orange", "orange", "black"]        
    },
    "expected": {
        "label": "33 ohms",
        "value": 33
    }
}

Encoding the components of a composite return value is the closest thing to what I’d say is the “best compromise between extremes” after #1553, so I’d prefer something like that. I would propose to tweak your suggestion slightly into something like this:

{
    "description": "Orange and orange and black",
    "property": "label",
    "input": {
        "colors": ["orange", "orange", "black"]        
    },
    "expected": {
        “value”: 33,
        "unit": "ohms"
    }
}

This way the implementing language can build whatever label that is most appropriate to both the language and where this language gets placed in the progression for that language. As it’s just the final state attributes of a generic representation a language can, relatively easily — and more easily than with an already encoded composite label — make the representation under test be a composite string, a struct or tuple, or whatever object type is most appropriate.

Having the “33 ohms” label in there as a third key would make this easier to implement for those who want this exercise to end in that string, but would make this less generic to the notion of composite return values, which are a generic problem.

End result is something not ideal for anyone, but which maximizes utility for all sides.

@ErikSchierboom
Copy link
Member Author

ErikSchierboom commented Jul 24, 2019

What do you guys think of this: use proposal 1, but include in the comments a jq filter which merges the value inputs into the label outputs, and gets rid of the extraneous value inputs

And by this you mean have separate properties, right? If so, what do you think about my objections to this approach (tightly-linked, duplicate value/label test case combinations required)? I'm especially hesitant to tightly-link the test cases.

@yawpitch I like the "ohms" unit suggestion:

{
    "description": "Orange and orange and black",
    "property": "label",
    "input": {
        "colors": ["orange", "orange", "black"]        
    },
    "expected": {
        "value": 33,
        "unit": "ohms"
    }
}

End result is something not ideal for anyone, but which maximizes utility for all sides.

Yep, it is definitely a compromise, but if it's workable for all then maybe it is the best we can do. @coriolinus could you live with this compromise?

@coriolinus
Copy link
Member

Yes, that compromise works well for me, because the struct fields are orthogonal. We haven't decided what to do about choosing units for larger values. There are two options:

    "expected": {
        "value": 33000,
        "unit": "ohms"
    }
    "expected": {
        "value": 33,
        "unit": "kiloohms"
    }

The first is more general. The second better fits the original intent of this exercise. I mildly prefer the first, but I am willing to accept either.

@coriolinus
Copy link
Member

To actually answer the question asked: I agree that tightly linking the test cases isn't great, but it's necessary in order to be able to apply an automatic jq transform. I think the proposed compromise is a better idea.

@ErikSchierboom
Copy link
Member Author

We haven't decided what to do about choosing units for larger values. There are two options:

My preference would also be with the first option, as that is indeed the most general.

@yawpitch
Copy link
Contributor

yawpitch commented Jul 24, 2019

Yes, that compromise works well for me, because the struct fields are orthogonal. We haven't decided what to do about choosing units for larger values. There are two options:

    "expected": {
        "value": 33000,
        "unit": "ohms"
    }
    "expected": {
        "value": 33,
        "unit": "kiloohms"
    }

The first is more general. The second better fits the original intent of this exercise. I mildly prefer the first, but I am willing to accept either.

My instinct is to go for the more general approach, but in this case it strongly serves the special case that inspired the exercise well to have it be “kiloohms”, so I’d tend to say that approach would perhaps be a better fit for this exercise’s needs. Mild tangent: if I read the description correctly three bands can encode up to 99,000,000,000 ohms... shouldn’t we have tests for megaohms and gigaohms as well? Even if only optional “bonus” ones? If optional should we consider dekaohms and hectoohms as well, for SI consistency?

@ErikSchierboom
Copy link
Member Author

ErikSchierboom commented Jul 24, 2019

@F3PiX @SleeplessByte @iHiD what are your thoughts?

@coriolinus
Copy link
Member

shouldn’t we have tests for megaohms and gigaohms as well? Even if only optional “bonus” ones? If optional should we consider dekaohms and hectoohms as well, for SI consistency?

That sounds like a good idea to me.

@ErikSchierboom
Copy link
Member Author

That sounds like a good idea to me.

I think that is a good idea. I'll add some optional values tonight, and update the canonical data as well. My thinking now is that @yawpitch has it right, and it perhaps better to use "kiloohms' as the unit.

We're getting there! Thanks everybody for being so responsive and chipping in.

@emcoding
Copy link
Contributor

I'm glad that there's a solution.
I do have 2 questions:

  • Is this really easier for maintainers than the thing I proposed: splitting the requirements into 2 different exercises, where this Resistor III returns a label, and Resistor IV is going to return values only [3300, 20]? Maybe it's naïve thinking, but I'd think it's easier for maintainers to skip III and implement IV, or the other way around, than having to do the extra work.
  • Is it really important that an exercise doesn't fit other tracks? That is news to me, because there are multiple exercises that don't fit the Ruby track. I would never expect that to be a reason not to implement that exercise, if it's expected to be useful to at least 1 or 2 other tracks.

@yawpitch
Copy link
Contributor

We're getting there! Thanks everybody for being so responsive and chipping in.

@ErikSchierboom I do believe we are getting there, but I think this fix might require an update to the canonical schema, since right now expected seems to only allow for either a single value or an error.

@ErikSchierboom
Copy link
Member Author

@yawpitch That won't be necessary, we already have exercises returning objects (e.g. ETL).

@yawpitch
Copy link
Contributor

yawpitch commented Jul 24, 2019

@F3PiX

Is this really easier for maintainers than the thing I proposed: splitting the requirements into 2 different exercises, where this Resistor III returns a label, and Resistor IV is going to return values only [3300, 20]? Maybe it's naïve thinking, but I'd think it's easier for maintainers to skip III and implement IV, or the other way around, than having to do the extra work.

That is, as you say, naive ... sure, splitting the exercise here would eliminate the headache for now, but that is just a recipe for fracturing in the future when this comes up again, and again, and again. And it will. So you'd get exponential growth in the number of exercises as the complexity of those exercises goes up, and most of the difference between them would be boilerplate. A general solution for encoding the most reduced form of a composite answer is going to reduce that growth and keep the core job of the maintainers (implementing new exercises for their track and maintaining existing exercises as canonical data changes) to a minimum.

Is it really important that an exercise doesn't fit other tracks? That is news to me, because there are multiple exercises that don't fit the Ruby track. I would never expect that to be a reason not to implement that exercise, if it's expected to be useful to at least 1 or 2 other tracks.

Yes, to some of us, it is really that important ... this debate wouldn't be happening if that wasn't a strongly held value and priority of at least some of the maintainers, to say nothing of the mentors and students who don't have a voice here.

Broadly speaking any exercise that Ruby can address can be equally addressed by every other imperative (or multi-paradigm) language (which IIRC constitutes a majority, but at the least a very significant block of, the languages we currently support) ... the only exercises, really, that do not and cannot apply to a language are those that are specific to one of the major paradigms (ie OOP, Functional, Declarative) that a given language does not support. And for each of those (except for wherever Prolog fits), there's overlap with an entire family of languages. Consider for instance Julia to be a language in which anything you want to teach in Ruby has a necessary and valuable equivalent lesson to be taught, hence an equal amount to gain from the canonical data ... but only if the canonical data isn't hand-crafted to a track-specific outcome.

In fact I'd say that an exercise probably should not have a canonical representation unless and until it's been normalised to a form that applies to its paradigmatic family of exercises ... which means that likely only Prolog can really not think about this issue as important.

Edit before anyone gets miffed at my picking out Prolog above, it's just the only language I can think of in our list of supported langauges that seems to exist in a paradigm on its own.

@yawpitch
Copy link
Contributor

BTW, one of the key and unique value propositions of Exercism, from my point of view as a would-be polyglot programmer, is the ability to attack the same problem in a dozen or more different languages, just to see how those languages approach the same issues. I don't believe that Exercism should become 50+ different CodeAcademy clones ... IMHO that would be best served by promoting to canonical those exercises that both can and -- where reasonably applicable -- should be implemented widely.

Admittedly that's a bias, but it's a bias based on what drew me to Exercism in the first place, before the addition of mentoring. I just don't want to see that value unnecessarily degraded by what I see as a premature and unconsidered willingness to think of the canonical data as a 2 or 3 language specific data store.

@emcoding
Copy link
Contributor

shouldn’t we have tests for megaohms and gigaohms as well?

The current test values all exist in real 3 band resistors.
As far as I could find, gigaohms resistors exist, but with 4 bands, not 3.
megaohms do exist as a 3 band resistor.

Two options

  • We can save the gigaohms for the next exercise in the series, that will handle 4 bands
  • We can add the extra cases if it's okay that maybe they don't exist as real resistors.

@yawpitch
Copy link
Contributor

shouldn’t we have tests for megaohms and gigaohms as well?

The current test values all exist in real 3 band resistors.
As far as I could find, gigaohms resistors exist, but with 4 bands, not 3.
megaohms do exist as a 3 band resistor.

Two options

* We can save the gigaohms for the next exercise in the series, that will handle 4 bands

* We can add the extra cases if it's okay that maybe they don't exist as real resistors.

I'd tend to think that, rather like roman-numerals and leap, we can assume that the result of the exercise need not refer back to a resistor that is actually manufactured. That said it'd be quite reasonable to say in the description.md that there's some upper limit beyond which you simply wouldn't practically use a 3 band resistor, even if you could actually encode values above that limit, and thus we won't test beyond it.

@ErikSchierboom
Copy link
Member Author

I've updated the PR to use the new, agreed-upon format for the expected value. Also, I've tried to add the optional test cases, but there is no precedent for having optional test cases, and the existing keys don't really make much sense. We can of course fix this, but I'd prefer doing that in a separate PR when the need arises.

Once again, thanks for the comments!

@yawpitch
Copy link
Contributor

the existing keys don't really make much sense.

I don't mind this going out without those tests, just thought this was an ideal case for the optional flag. As I understood it that string is just arbitrary though, isn't it? So basically a comment and the only reason for OPTIONAL-KEYS.txt is to keep those comments from -- potentially -- duplicating out of control? This is such a generic case that "humanized-outputs" or "extended-outputs" would probably cover it.

Copy link
Contributor

@yawpitch yawpitch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really glad to see this version.

@ErikSchierboom ErikSchierboom merged commit c0ecfc2 into exercism:master Jul 24, 2019
@ErikSchierboom
Copy link
Member Author

Merged! Thanks all. 🎉

@ErikSchierboom ErikSchierboom deleted the resistor-color-trio branch July 24, 2019 17:03
@ErikSchierboom
Copy link
Member Author

I don't mind this going out without those tests, just thought this was an ideal case for the optional flag. As I understood it that string is just arbitrary though, isn't it? So basically a comment and the only reason for OPTIONAL-KEYS.txt is to keep those comments from -- potentially -- duplicating out of control? This is such a generic case that "humanized-outputs" or "extended-outputs" would probably cover it.

Yep, it is a great candidate. "humanized-outputs" or "extended-outputs" sounds good. @sshine what do you think would be a good, generic value for this optional value?

@sshine
Copy link
Contributor

sshine commented Sep 10, 2019

@ErikSchierboom: I didn't see this comment until now, sorry!

I understand that your question here is whether I think "humanized-outputs" or "extended-outputs" are good candidates for OPTIONAL-KEYS.txt. In general, I don't know. In this particular exercise, I don't think so personally:

If I were to port this exercise to one of the tracks I participate in currently:

  • Haskell: Has no test generator, would create a newtype Resistance = Resistance { ohms :: Int } definition for the return value and ask the student to provide an instance Show Resistance where show = ... that produces something alike that in the canonical tests, and write the test suite manually based on the canonical tests, as is the custom on the track. So, same task, but split into parts.

    Come to think of it, I think this is actually an excellent way to introduce custom Eq, Ord and Show instances rather than derived ones. So a type class-centric exercise. I like it! Also, it will be fun to have an exercise called "-trio" when the two others aren't there. :-D

    This reminds me of a 1990s Danish comedy trio where two of the comedians didn't show up on stage until a while into one sketch, because they were out shopping. The show was called "Where are the two others?" -- I think it might've been a pun on the fact that they got asked that a lot in private.

  • OCaml: Has a test generator, would create a data type definition for the return value, and an exercise-specific JSON deserializer for the canonical tests, so that there's a translation between the JSON object in the canonical test and the data type in the OCaml exercise test suite. I would probably accept that values of this data type does not have a canonical form (so testing that 33000 ohms and 33 kiloohms for equality cannot be done with = without first normalizing it), or try to achieve something alike the Haskell solution using module-level functors. If it weren't because F# has units of measure, this is probably similar to how I'd do it in F# -- do you think?

  • Tcl: Has no test generator (yet), but regardless of this, I would probably return a list {33 ohms}. Tcl has coercion between strings and non-nested lists, so you can print or index that without problems. On the whole "but is 33000 ohms the same as 33 kiloohms?" debacle I just made up: I would say that I don't care, because if you cared about such things, you wouldn't use Tcl anyways. :-P

@yawpitch wrote:

before anyone gets miffed at my picking out Prolog above, it's just the only language I can think of in our list of supported langauges that seems to exist in a paradigm on its own.

Yeah, I think you'd call it logic or constraint programming, and I've been taught to sub-classify it as declarative, just like FP. When I was taught Prolog at school, we were asked not to use the ! cut operator, as Wikipedia reiterates: "*Prolog is not purely declarative: because of constructs like the cut operator".

I hope this is not miffed, but I can think of two better examples:

  • The Ballerina language, which mostly made up its own exercises. I haven't made an effort to classify Ballerina's programming paradigms, and I don't know if they deviated from canonical exercises because they had to, or because they wanted to.
  • I've wanted to make a jq track (to learn the damn thing). Since it's a programmable query language, I'd hardly expect any overlap with canonical exercises.

@coriolinus
Copy link
Member

coriolinus commented Sep 10, 2019 via email

@ErikSchierboom
Copy link
Member Author

Thanks for the detailed response @sshine! I think that the original question you responded to has been partially voided later on, as the structure of the returned data was modified after said question. It's great to hear that you see value in implementing this exercise in other languages.

sshine added a commit to exercism/haskell that referenced this pull request Nov 9, 2019
This adds the exercise from exercism/problem-specifications#1551.

In addition to the canonical exercise, this implementation extends:
 - Add a "zero" test case (Black, Black, Black).

 - Add a `Data.Text` hint in `.meta/hints.md`.

 - Export both a `label` function *and* an `ohms` function: While the
   exercise is mainly about printing a formatted string, this nudges
   the student towards separating print and calculate.

 - For totality, since `Resistor` allows expressing resistors in the
   megaohm and gigaohm ranges, extend the exercise so that the correct
   unit beyond kiloohms are returned.

 - Extended unit tests are added, and coverage is described as such:

   Canonical tests are marked with 'x'.
   Extended tests are marked with 'y'.
   Optional (commented out) tests are marked with 'z'.

   ```
   Black       xyyyyy    xyy
   Brown       xyz       x
   Red         xyz       xz
   Orange      xxz       x
   Yellow      xyz       x
   Green       xyz       yz
   Blue        xyz       y
   Violet      xy        y
   Grey        xy        yz
   White       yy        y
   ```

   The over-representation of Black is partly caused by the fact that
   canonical and extended tests avoid edge cases like "4.7 kiloohms".
   There is some didactial point in incremental improvements, and as
   such, canonical and extended tests partially validates solutions.

   Optional (commented out) tests make the exercise considerably more
   difficult, so they are accompanied with a disclaimer that students
   may choose freely to include them or not. Separate them visually so
   maintenance is easier.

   This way, students who are oblivious to the edge cases will not be
   aware of them, and students who are not (either by thinking or by
   inspecting the unit tests) will be able to choose.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new exercise idea Track Anatomy Project Related to or helpful to the Track Anatomy Project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal new exercise in the Resistor series: Resistor Color Trio
10 participants