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

baseapp: Functional options should use setters and seal #1645

Closed
ValarDragon opened this issue Jul 11, 2018 · 3 comments
Closed

baseapp: Functional options should use setters and seal #1645

ValarDragon opened this issue Jul 11, 2018 · 3 comments

Comments

@ValarDragon
Copy link
Contributor

ValarDragon commented Jul 11, 2018

Currently, our initialization uses functions on un-exported variables. This means that all possible functions we'd ever want to be defined for initialization must be defined in the baseapp module. Therefore if there is some fancy type of initialization I want as an end user on the variables set in baseapp, I have to mod the sdk. I don't think we should limit what end users can do like that. Note, several of the unexported functions do have this already. E.g. SetTxDecoder, SetAnteHandler. (Albeit not with the proposed seal method)

Instead I propose we have setter variables on each of the unexported variables, and a Baseapp.Seal() method. Once the Baseapp.Seal has been called, setters no longer set. I think this will allow for greater extensibility of baseApp.

@alexanderbez
Copy link
Contributor

We definitely need a Seal method. Can you elaborate on "setter variables"? Maybe give a trivial example?

@ValarDragon
Copy link
Contributor Author

Woops, I meant to write setter functions there.

@rigelrozanski
Copy link
Contributor

Great design pattern - we can make it crystal clear in the code what the seal does

@ValarDragon ValarDragon mentioned this issue Jul 19, 2018
22 tasks
@mossid mossid mentioned this issue Jul 25, 2018
9 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants