Exercise ResistorColor is confusing or too difficult

[jhaand]

Hi,

For the C track I found the last test of this exercise very confusing.
It was very easy to map the mnemonics to an enum for the first 3 tests.

https://exercism.org/tracks/c/exercises/resistor-color

But the last test has no signature for the colors() function. And the solution requires to return an array of values on the heap. (static array) I find these quite advanced topics for an easy assignment without any hints.

Which invites people to look up solutions.

It would actually have been easier for me to calculate the resistor value based on 4 random colors.

[github-actions]

Hello. Thanks for opening an issue on Exercism. We are currently in a phase of our journey where we have paused community contributions to allow us to take a breather and redesign our community model. You can learn more in this blog post. As such, all issues and PRs in this repository are being automatically closed.

That doesn't mean we're not interested in your ideas, or that if you're stuck on something we don't want to help. The best place to discuss things is with our community on the Exercism Community Forum. You can use this link to copy this into a new topic there.

---

Note: If this issue has been pre-approved, please link back to this issue on the forum thread and a maintainer or staff member will reopen it.

[ryanplusplus]

@jhaand the last test doesn't require you to return an array of values from the heap. The tests communicate this by not freeing the result of colors().

Can you help me understand why creating the colors() signature is challenging? The function takes no arguments and the line in the tests where it is used give the result type so I would have guessed that it would be straightforward to construct the signature by copying the result's type.

[siebenschlaefer]

@ryanplusplus I've experienced the same with other students in this exercise. Quite a few were asking how to solve the last test, others returned a pointer to a local variable asking why they got an error, a third group dynamically allocated the array with malloc().
I guess many of them are coming from languages with automatic memory management, or are just new to the language and not yet used to object lifetimes and the difference between local and static (or global) variables. That makes this last test quite a challenge when they have no idea where to start or get an error that is hard to understand and/or research.

[wolf99]

Which invites people to look up solutions.

I don't think this is a bad thing? Researching how others have solved a common problem is a valid part of software engineering.

I'll agree that having the signature could make this easier, but students need to learn it t some point. With the concepts part of the C track not yet in place we don't have a good way of teaching new things aside from, "here's something new, have a go at it".

Or to put it another way, students will need to face each hurdle at some point.

Having said that I would be interested in any proposals of an alternative resistor_color interface!

[jhaand]

@jhaand the last test doesn't require you to return an array of values from the heap. The tests communicate this by not freeing the result of colors().

Can you help me understand why creating the colors() signature is challenging? The function takes no arguments and the line in the tests where it is used give the result type so I would have guessed that it would be straightforward to construct the signature by copying the result's type.

I would expect to use an application to determine the actual resistor value. By testing the actual bands of a resistor. If you add the multiplier colors, that wouldn't be so hard. A test of:

const resistor_band_t resistor_bands = {YELLOW, VIOLET, GREEN,RED} 
TEST_ASSERT_EQUAL_UINT16(47500, colors(resistor_bands)); 

would make a lot more sense and lie closely to an actual application.

[wolf99]

I would expect to use an application to determine the actual resistor value. By testing the actual bands of a resistor. If you add the multiplier colors, that wouldn't be so hard. A test of:

const resistor_band_t resistor_bands = {YELLOW, VIOLET, GREEN,RED} 
TEST_ASSERT_EQUAL_UINT16(47500, colors(resistor_bands)); 

would make a lot more sense and lie closely to an actual application.

This is essentially what the later resister color exercises do. This first exercise starts with single color input so as not to require passing in multiple args, e.g. as an array. The later exercises then build with that aspect.

[jhaand]

This is essentially what the later resister color exercises do. This first exercise starts with single color input so as not to require passing in multiple args, e.g. as an array. The later exercises then build with that aspect.

The test with colors() does not make sense.
I have no problem with passing an array as argument. But then it should have normal value. Not just {0,1,2,3,4,5,6,7,9,0} which is a bogus value. Make it mean something. As for the return value. Make it a scalar value or make clear that it should return a pointer to an array that remains outside the scope of the called function.

[jhaand]

Which invites people to look up solutions.

I don't think this is a bad thing? Researching how others have solved a common problem is a valid part of software engineering.

I looked up other peoples solutions in a private window in order to get an idea of what was required. I think that Exercism shouldn't require such an approach.

[wolf99]

Make it mean something. As for the return value. Make it a scalar value

That's exactly what color() does
Example: color(ORANGE) # returns 3.

colors() returns all the possible colors.
It is a sort of convenience function in that sense.

Possibly two options to help with this:

1. Rename the test names so that they give more meaning, for example:
   1. test_orange_is_interpreted_as_3()
   2. test_colors_returns_all_possible_colors()
2. Remove colors() and test_colors().

Both of these options would need to be implemented in the problem-specifications repo first before they could be updated here.

[wolf99]

Which invites people to look up solutions.

I don't think this is a bad thing? Researching how others have solved a common problem is a valid part of software engineering.

I looked up other peoples solutions in a private window in order to get an idea of what was required. I think that Exercism shouldn't require such an approach.

I don't see a problem with this. Technically students can find the solutions online and just copy-paste all of them if they wanted to. If students want to learn a language rather than learn how to copy-paste or learn how to be spoon-fed, they need to put in at least some research

[dlesnoff]

The static array solution could go in a Resistor Color II problem.
The fact that this exercise is the third one is really discouraging for the rest of the track.
I managed to look up some code and find that it requires a static array. It should not invite people to search for the solution of this exercism problem online, otherwise they will only cheat or search direct solutions without thinking first.

[fapdash]

Maybe it helps if I explain the way I solved this exercise. I just finished this exercise as C newbie with very little experience in languages that force you to do manual memory management.

1. I looked at the test code, I saw that it expects an array with all the colors:
   

   c/exercises/practice/resistor-color/test_resistor_color.c

   Lines 34 to 36 in 78f9e85

   	const resistor_band_t expected[] = { BLACK, BROWN, RED, ORANGE, YELLOW,
   	GREEN, BLUE, VIOLET, GREY, WHITE };
   	TEST_ASSERT_EQUAL_INT_ARRAY(expected, colors(), ARRAY_LENGTH(expected));

2. After learning that I tried to return an array from the function, the easiest way I could think of was to copy the code from the test:

   resistor_band_t *colors() {
     resistor_band_t expected[] = { BLACK, BROWN, RED, ORANGE, YELLOW, GREEN, BLUE,  VIOLET, GREY,   WHITE };
     return expected;
   }

   This led to an error: error: function returns address of local variable
3. After googling that message and the first Stack Overflow question not being that helpful I decided to google for "C returning array from function". This led me to https://www.tutorialspoint.com/cprogramming/c_return_arrays_from_function.htm which reads:

   Second point to remember is that C does not advocate to return the address of a local variable to outside of the function, so you would have to define the local variable as static variable.

4. The site didn't explain exactly what a static variable does so I googled "C static variable", the first result was https://www.geeksforgeeks.org/static-variables-in-c/ which talked about all sorts of stuff related to memory management and whatnot that I didn't fully understand while skim reading and didn't want to take the time to properly read so early in the track. So I decided to just slap static on the variable and run the tests again:

   resistor_band_t *colors() {
    static resistor_band_t expected[] = { BLACK, BROWN, RED, ORANGE, YELLOW, GREEN, BLUE,  VIOLET, GREY,   WHITE };
    return expected;
   }

   -> test is green

I think the Exercise is fine on it's own, the amount of research I had to do is what I expect from a track without a syllabus. Would I expect it so early in the track? No, I'd propose to move the exercise a bit more to the back. But then again, I'm a C newbie so not sure if there are easier exercises to put there that don't force you to learn about memory management.

[ryanplusplus]

I think the Exercise is fine on it's own, the amount of research I had to do is what I expect from a track without a syllabus. Would I expect it so early in the track? No, I'd propose to move the exercise a bit more to the back. But then again, I'm a C newbie so not sure if there are easier exercises to put there that don't force you to learn about memory management.

Thanks for the feedback. I think moving the exercise a little later is a good balance. Given that we don't have concept exercises, someone working through the track is going to have to do a bit of research to get past this, but pushing it a bit later is an easy change.

[ryanplusplus]

Please have a look at this proposed change:
#856

This bumps the difficulty from 1 to 3 and moves it to the end of the exercises with the same difficulty rating.

[jhaand]

I solved it in the same manner as @fapdash did. The solution @ryanplusplus proposes looks good to me.
The exercise does pose an interesting problem to solve at a higher difficulty.

[siebenschlaefer]

Just a small idea: What if would split the line

TEST_ASSERT_EQUAL_INT_ARRAY(expected, colors(), ARRAY_LENGTH(expected));

into two lines:

const resistor_band_t *actual = colors():
TEST_ASSERT_EQUAL_INT_ARRAY(expected, actual, ARRAY_LENGTH(expected));

That would make it more obvious to beginners that colors() should return a pointer.

The very tiny downside is that this would enforce a pointer to resistor_band_t as the return type, const resistor_band_t (*colors())[10] where the function returns a pointer to an array of length 10 would no longer be possible. But I haven't seen this approach in any solution so that should not be a real problem.

[emaphis]

Or create an earlier unit test that simply tests that colors() must return a pointer. That way a helpful error message can be returned/

[siebenschlaefer]

@emaphis That won't work because before test can run and print a helpful message you will get a compiler error if you choose the wrong return type.

[emaphis]

@siebenschlaefe Ok. But if the student fixes unit test compile errors systematically from top to bottom, the will run into the `ColorsMustReturn APointerToAnArray or whatever earlier on.
In other words, the unit test is more of aa hint than a real test.

[siebenschlaefer]

We don't have these "hint-like" unit tests anywhere else in the track, so I'm not sure if that's the way to go.
And the two lines from my comment above would accomplish the same things, wouldn't they?

[emaphis]

Yes it would, the purpose of the line to highlight api type errors is not very explicit though. Maybe if you include a comment:

const resistor_band_t *actual = colors(): // get this line to compile

Students would know that line is a hint instead of a implementation artefact.

[ryanplusplus]

@siebenschlaefer, I like your suggestion. I'll update my PR to include those changes.

@emaphis I think it will probably be clear enough without explicitly noting that the line should compile. We can revisit if we continue to have issues.