-
-
Notifications
You must be signed in to change notification settings - Fork 7
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
Add resistor-color-trio exercise #53
Conversation
Fix spacing so methods are 2 empty lines apart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. @pfertyk, at some point we should sort the exercises by difficulty group as shown on the website (I believe it's 1-3 for easy, 4-6 for medium, 7-10 for hard) and then by exercise name within each group. The reason I mention this is that the resistor-color series is a notable exception where it'd be better to sort them sequentially. That lets the student see how each step leads into the next one from resistor-color to resistor-color-trio
Another thing to float. Do we want the inputs to be arrays of colors or do we want to require three arguments, one for each color. The latter would make the last test about ignoring additional colors unnecessary. All the other tests use three colors. At the moment, the ported resistor-color-duo uses an array of colors so we should be consistent, but that doesn't preclude us from changing resistor-color-duo to use two arguments, one for each color. |
@BNAndras noted, I already try to sort the exercises by difficulty, but we can update the whole list later 👍 As to arrays vs separate arguments, I am OK with both solutions. I would, however, lean a bit toward an array, as it will be consistent with the Python track. GDScript is very similar to Python, and I like to keep the solutions similar to make things easier for people who already completed the exercises in Python. On the other hand, separate arguments will be "consistent" with the first exercise of this series (which accepts a single argument, not a single element array) :P What are your thoughts @glaxxie ? |
Yeah, I wouldn’t have thought of it but the Julia maintainers mentioned it when I ported resistor-color-duo. They still ended up using an array because resistor-color-trio was already ported that way. I think the array approach is pretty typical from what I’ve seen in other tracks. |
Fixed the spacing issue. I didn't use the godot editor at first since I was fiddling about for far too long, so I used vscode for the first PR. Regarding the point about using array vs args: I would favor array as well for the subsequent resistor exercises. I actually did have a conversation related this when code review with a learner. They asked why in the From that, we can rationalize that the very first exercise is more about introducing the idea of assigning a color to retrieve a certain value so using one arg (string) here is ok, and not really an accurate portrait of how resistor should be like later ones (array of colors), just like there isn't one with a leading black color (0) irl so there is no need to test for them either. Edit: I also have the |
Yeah, that GitHub thing led to a few issues for me on the first few PRs until I got the hang of it. :) |
Fix another spacing issue
Thank you for contributing @glaxxie ! :) |
First timer 🙏