Help with reviewing a PR adding an approach

I got a PR for a new approaches article on the Rust track. (I guess these are intentionally excluded from the auto-closing mechanism.)

I’m not sure how to review this. The exercise is reverse-string. My gut feeling says this exercise is too trivial to warrant an approaches article. But maybe that’s not the right way to think about it? The documentation says:

(the approache’s) contents should either:

  • Explore an idiomatic approach
  • Explore a non-idiomatic, but interesting approach

The idiomatic solution is a one-liner and I don’t find the other approaches particularly interesting.

I’d appreciate some thoughts about what to do here.

Looking at the PR, I definitely think over-engineered should not be one of the approaches. Not sure if the recursion approach is anything people would use in Rust. The iterators vs iterative approach could be useful when pitted against each other, or when one is favored over the other, to explain why. I like the graphemes one, although I feel that’s more a special-case of the iterators approach.

That approach is very unidiomatic. Both out-parameters and recursion. Recursion because it is not necessary like in functional languages and tail-call optimization is not guaranteed by LLVM. So that approach would have to go under “unidiomatic but interesting” category if it is to stay.

The collect version is always favored. It is more readable and usually has more optimizations by default, e.g. reserving the necessary amount of memory up-front if the number of items in the iterator is known.

I’d then remove it.

Then I’d probably only list the collect version.

In that case, we’re only left with the collect approach and maybe the grapheme cluster version, right? Both of which are intuitive one-liners. Is it worth having an approaches article for that?

Just checked the community solutions and as I expected, 90% of them are either of these two one-liners.

Well, there’s still 1 in 10 that didn’t get that, so I do think there is value in it.
I don’t think the collect and graphemes versions are different approaches, the latter is more of a tweak to the former.

What is the value of such an approaches article above just looking at the community solutions? In my mind, approaches offer two advantages over just looking at the community solutions:

  • Extracting the essential, interesting differences between solutions using different approaches. The community solutions may be chaotic with content that mixes different approaches or some hidden gems might be far down the list.
  • Providing context and information about the differences between these essential approaches.

Neither seems to be the case here. The community solutions are very homogenous and there isn’t much context to give for the one idiomatic solution.

Then you might be better with an article than an approach.

Articles can explore all this, but approaches are designed to be things that solutions are tagged with. So users completing one approach will be sign-posted to try out the other valid approaches too.

Alright, that makes sense. What would be the topic of the article?

In my mind there is a third advantage:

  • Listing idiomatic/authorative solutions. Sure, one can browse the community solutions, but having it listed in the approaches is validation that it is indeed an idiomatic solution.

IMHO, I would have just the one approach and then have a little section on Unicode support. That can all be done in the if needed.

I like that! Thank you.

1 Like