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

NodeRedux state container for ESP devices #3421

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

shubham-sri
Copy link

@shubham-sri shubham-sri commented Apr 9, 2021

Fixes #3420

  • This PR is for the dev branch rather than for the release branch.
  • This PR is compliant with the other contributing guidelines as well (if not, please describe why).
  • I have thoroughly tested my contribution.
  • The code changes are reflected in the documentation at docs/*.

@shubham-sri shubham-sri changed the base branch from release to dev April 9, 2021 17:36
@nwf
Copy link
Member

nwf commented Apr 10, 2021

Thank you for your contribution.

This feels very... heavy? Enterprise-y? for an embedded system. Just picking on one example, constructing the first error message in assertReducerShape, even with an empty key k will consume 295 bytes of RAM, credibly between .5% and 1% of the application's initial available heap!

The more idiomatic approach in Lua is to use its OOish facilities, specifically, table lookup (. or the : sugar) and function call as message send. While you could do that inside your store's reducer function to avoid the if/elseif/else chains, it's no less flexible or compositional to instead pass the store itself as a table to the would-be message sources and have them send messages by invoking particular fields. The use of metatables even affords opportunity for "unknown message" handling, should that be useful. If isolation is desired (though it is often less of a concern with the size of applications that fit in ESPs), the store methods can close over the mutable state rather than holding it in the store table itself (though this is not without its costs: the store methods become closures rather than static function references).

@shubham-sri
Copy link
Author

shubham-sri commented Apr 14, 2021

Thank you for your contribution.

This feels very... heavy? Enterprise-y? for an embedded system. Just picking on one example, constructing the first error message in assertReducerShape, even with an empty key k will consume 295 bytes of RAM, credibly between .5% and 1% of the application's initial available heap!

The more idiomatic approach in Lua is to use its OOish facilities, specifically, table lookup (. or the : sugar) and function call as message send. While you could do that inside your store's reducer function to avoid the if/elseif/else chains, it's no less flexible or compositional to instead pass the store itself as a table to the would-be message sources and have them send messages by invoking particular fields. The use of metatables even affords opportunity for "unknown message" handling, should that be useful. If isolation is desired (though it is often less of a concern with the size of applications that fit in ESPs), the store methods can close over the mutable state rather than holding it in the store table itself (though this is not without its costs: the store methods become closures rather than static function references).

Hey @nwf, thank you for the nice suggestion

The reducer function is created by the end-user so for more flexibility I have to add if/elseif/else chains, and this same is used in Redux.

And assertReducerShape can be optimized, I'm looking to solve it in an efficient way.

@nwf nwf marked this pull request as draft June 3, 2021 12:16
@stale
Copy link

stale bot commented Jun 12, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Redux state container for ESP8226, ESP32
2 participants