Skip to content
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 another assert to Agent "maintain state" koan #219

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jordan-brough
Copy link

To clarify/emphasize that this is a persistent state.

To clarify/emphasize that this is a persistent state.
Copy link
Collaborator

@iamvery iamvery left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion!!

Can you tell me a little more about how you feel this improves on the understanding of this lesson?

@jordan-brough
Copy link
Author

When I first saw the code it wasn't 100% clear to me whether the result was a one-time thing or not. I wrote this extra test to confirm to myself that the state would remain persistently, and thought perhaps it would be a helpful thing to have by default.

@iamvery
Copy link
Collaborator

iamvery commented Mar 27, 2018

Hmm ya, I see what you mean. I feel like even this example might lead the learner to believe that the function is executed each time rather than being a producer of the state. Do you think a later example that shows how the state can be updated serves to solidify that better than a repeated assertion?

Thoughts @felipesere ?

@jordan-brough
Copy link
Author

For me this koan was confusing because the text said "maintain state, so you can ask them about it" but the example only queried a single time so the persistence wasn't clear. Especially coming fresh from the previous Processes and Tasks modules (where messages and responses are one-time events) this aspect was confusing to me initially. It would be nice to have something that clears that up imo. The later examples do but I was confused about that initially.

@felipesere
Copy link
Collaborator

Thank you @jordan-brough for opening this PR 👍

Should we maybe connect this Koan somehow with one where we explicitly mutate the state?
Its kinda hard to prove that the state won't change. Looking at it now (having done a bunch of Elixir since the Koans) it feels natural that nothing changes unless instructed to do so.
Maybe your connection to processes and messages also gives us a clue? Maybe we can make the difference more explicit?

@iamvery I also feel we could extract the function in start_link itself into something like
one_off_producer = fn -> state end and lift state into its own variable?

@iamvery
Copy link
Collaborator

iamvery commented Mar 28, 2018

@felipesere i like that! let the code communicate that intent 💪

@jordan-brough
Copy link
Author

Sounds good! I'll take a stab at refactoring along those lines. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants