[react] Refactor API to be less open

I think the existing API makes it easy to have a brittle implementation that fails when react is used as a package.
The proposed change aims to unexport some of the interface as a minimal change.


Frankly, I think it is a bad change; my hope is that this PR will be something else entirely before approval, hoping to have the discussion here, but perhaps an issue is more appropriate.

I think a better change would look like an API that doesn’t secretly rely on coupling across private types that expose a public only interface.
I’m not certain that this API is unsatisfiable to extend, but I’m suspicious that it is.
Looking at other solutions, I should be able to take someone’s Reactor, [Input|Compute]Cell and get the interface to work, but I’ve not been able to do so.

I think I need someone more experienced to tell me if this is

  • a correct analysis
  • useful thing to notice for learning Gophers
  • useful in the context of Exercism

I think the core of the issue is that none of the Cell types can express state change through the API, but any implementation must accept an arbitrary Cell as an upstream.
I’d be happy to be wrong, but neither mentor nor I figured it out yet, but my mentor, IsaacG did note that it’s pretty low value, especially with the proposed changes.

So my question might be pushing toward if/how exercism teaches a bit about API design in the Go track. I think it’s something like the open-closed principle?

Note: I’m a learner on the track, so I might be totally off base.

1 Like

I did just realize that insertion order is a sufficient resolution order to iter cell in reactor.insertOrderCells[inputCellIdx:] { update and run all callbacks }

As one of the Go track maintainers, I don’t have a whole lot more to add that I didn’t already mention :slightly_smiling_face:

This is a practice exercise so it’s not specifically designed to teach/practice any specific design. That said, ideally it would be a bit cleaner and more rigorous by either unexporting classes or possibly updating the interface to require a way to access downstream cells.

However, this exercise isn’t particularly about practicing an API and this shortcoming doesn’t, as far as I know, hamper people from solving this exercise. While the interface is not ideal, I don’t think it’s bad enough to warrant breaking existing solutions.

1 Like

When I see a comment like this, my first thought is typically whether it’s worth making a separate concept exercise rather than retrofitting an existing exercise. It’s a bit more work but you have a lot of more flexibility since you don’t need to follow an upstream source of canonical test data that’s tailored to work across multiple tracks and can go deep on the Go-specific nuances.

Considering I seem to have been the only student tripped up by broadening the context of a public API, I agree with y’all that we should not change the existing exercise’s code.

I also think that since I’m in the minority that adding text to the instructions about this aspect could be confusing as well unless it’s really well crafted - I don’t think I could do it - so touching the exercise as a whole seems unhelpful for new learners.

So a new concept exercise could help. It’s tricky for me to see since my trouble was mostly around the design of the API. Perhaps there’s a way to propose an exercise that is still testable? It’s certainly non-obvious to me.

@IsaacG watched me stumble around a barrier I couldn’t name for a while, is there anything you thought or think when rereading the chat “if this thing clicked, then Orion might move forward”?

Also, let me know if this should simply be another thread.

You (correctly) locked onto the fact that the public interface, as designed, had some limitations. I was (implicitly) suggesting you don’t worry about the given interface, and feel free to extend the API and/or assume the methods you write will only be called with Cells created with your code.

Would calling the out expliciitly in the instructions help?

My guess is that it would be difficult to add in a mention without it being noise for most; I’m the only one who has brought it up that we know of. The callout would have helped me, but perhaps only me.

But perhaps there’s a nice way to express that is is “an extra”, in that it’s not core to the topics in the exercise.

It may also open up naturally as a link to other materials on module structure and how things are exported. But the topic itself is less of a Go concept than an API topic.

Reviewing the conversation I had with Isaac, when he made this explicit like so, that’s what landed it.

For the purpose of this exercise you can assume all Cell objects are created by one of your Create*() functions ;)

As a remark at the end, especially with the wink, wouldn’t be noisy.

Thanks for what y’all do.

Would you like to open a PR to add that? Though I would cut emojis from the instructions. Maybe add a Note: or Hint: prefix instead?

1 Like

Sounds good, I added a short PR here [react] Update instructions to reduce focus on API design by YeungOnion · Pull Request #3136 · exercism/go · GitHub

1 Like

Thanks for the PR! I commented on your PR