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

fix brain API to be set/get or read/write, not set/read #359

Open
JorySchossau opened this issue May 27, 2021 · 2 comments
Open

fix brain API to be set/get or read/write, not set/read #359

JorySchossau opened this issue May 27, 2021 · 2 comments

Comments

@JorySchossau
Copy link
Member

setInput,readOutput should be setInput,getOutput or writeInput,readOutput for consistency.
Probably a consideration for next breaking update.

@cliff-bohm
Copy link
Collaborator

cliff-bohm commented May 31, 2021

Okay, so just a counter point.

read/write or set/get pairs usually are applied to systems where data is written to the system and then the same data is read from the system.

In defense of the current naming.

  • Set indicates setting the state of the brain before calling update.
  • then update is called
  • read indicates reading the resulting state of the output.

So, this language could indicate that this is not a simple read/write or set/get system...
Is this a "good" argument? Probably not... but it is an argument.

@JorySchossau
Copy link
Member Author

I think update is orthogonal to the naming of these setter/getter fields, because I can use them as standard setter/getter fields. I can have a series of complex if statements setting the input, then before I run update I can read out the inputs to see what the final input pattern is, possibly save to file, then call update as usual. So in this way they really do operate like traditional setter/getter fields that write a value and allow you to get the same value back.

I could see renaming update to computeOutputsFromInputs to make the relationship clearer. So maybe the following:

  • setInput()/getInput()
  • computeOutputsFromInputs()/update()/stepOutputsAndInputs()
  • getOutput()/setOutput()

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

No branches or pull requests

2 participants