reverse-string: Update function template to use a variable name instead of an underscore

[cbzehner]

Basically as-titled. It took me an embarrassing amount of time to figure out how to reference the _ in the current function template (i.e. change it to something else). I've really enjoyed the Rust track so far, just trying to help clarify what I found confusing. 😄

I tried to make the minimum number of changes here and ensured that _test/check-exercises.sh was able to validate the reverse-string exercise before submitting.

[coriolinus]

@cbzehner I'm not inclined to merge this as is; as I explained here, one of my goals as a track maintainer is to ensure that the stubs compile without warning as much as possible. Adding an unused variable to a function works against that goal.

However, I can see how the use of the underscore here might be confusing to someone new to the language. In my mind, this is a documentation issue. I'd like you to update this PR with some subset of the following changes:

• Add a section to .meta/hints.md explaining what underscore variables are and why they exist, and use configlet to regenerate the README from that. (Stack Overflow thread on this here.)
• Add an inline comment to the stub explaining underscore comments somewhat more briefly.
• If you feel that keeping a variable name in the stub is helpful, name it _input instead of input.

[cbzehner]

That's good feedback, thanks! Keeping the code warning-free seems like a reasonable goal here. How would you feel about passing the variable to unimplemented initially in order to silence the unused variable warning?

Something like:

pub fn reverse(input: &str) -> String {
    unimplemented!("Write a function to reverse {}", input);
}

[coriolinus]

Does that work? I've never seen unimplemented!() used like that, but if it compiles, I wouldn't object to that approach.

[ijanos]

It works! I'm surprised but I like this approach.

[coriolinus]

Looks good to me, thanks @cbzehner!

There are no pending change requests from anyone, so unless something is discovered in the interim, I'll plan to merge this on 2 March. If after that date I still haven't, please feel free to ping me to remind me.

[petertseng]

This question won't affect whether this PR should be merged: To what extent would we like similar changes to be made on any other exercises that contain a non-empty stub?

[coriolinus]

I'm happy to leave it on an as-needed basis, prompted by students filing issues and PRs. (Yes, I'm applying lazy evaluation to real-life responsibilities here.) Consistency is nice, but not essential in this case.

…

On Tue, Feb 27, 2018 at 10:18 PM, Peter Tseng ***@***.***> wrote: ***@***.**** approved this pull request. This question won't affect whether this PR should be merged: To what extent would we like similar changes to be made on any other exercises that contain a non-empty stub? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#438 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHdeTs2jqLBzz5f9YaMJFEEXNr3JB8zRks5tZHEQgaJpZM4SSflZ> .

[ijanos]

I think the underscore is arcane knowledge for a beginner, so I would love getting rid of it.

I prefer using a proper variable name and reference it in the unimplemented! macro so it will compile without warnings. The intention is clear and we can provide good variable naming examples.

I do not like the _name naming in a stub because we use it for warning suppression but I don't think this is obvious for a student. They can use that name without removing the underscore and solve the exercise with thinking the underscore is some weird rust thingy.

At least using a single underscore as a variable name may have the "this place is intentionally left blank" impression and it forces the student to change it because it is a compile error to use underscore as a variable.

[cbzehner]

Would y'all mind if I open an issue for updating the stubs?

I think it would be great to have it open and tag it as "beginner-friendly" to track which ones need to be updated and provide an entry point for people to get involved.

[coriolinus]

@cbzehner I would not mind that at all, particularly if you did the work of tracking down which exercises currently use underscores in the stubs.

[cbzehner]

Thanks y'all for all the feedback and help with this request. I know it's just as much work to review as to write this but I've learned a lot in the process and really appreciate it!