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

Rename AccountMapper to AccountKeeper #2540

Closed
jackzampolin opened this issue Oct 19, 2018 · 7 comments
Closed

Rename AccountMapper to AccountKeeper #2540

jackzampolin opened this issue Oct 19, 2018 · 7 comments

Comments

@jackzampolin
Copy link
Member

To avoid confusion. @sunnya97 is under the impression we retired the Mapper terminology.

@ValarDragon
Copy link
Contributor

I don't really like AccountKeeper. Mapper describes exactly what its doing. I think AccountKeeper is confusing, and only increases the barrier to understanding here.

@sunnya97
Copy link
Member

@ValarDragon What's the difference between a Mapper and a Keeper?

I think its easier just to merge these terminologies.

@ValarDragon
Copy link
Contributor

ValarDragon commented Oct 21, 2018

map is a fairly universal term throughout most of CS, and mathematics. The AccountMapper is a function which maps an address to an account.

The AccountKeeper is much less intuitive imo. Keeper does not have an intrinsic meaning which tells you what it does. This is fine for modules, where they have more complex functionality. I don't think this is a good idea for accounts, where there is a single purpose for this object.

@alexanderbez
Copy link
Contributor

I guess this begs the question, what cause for the initial confusion?

@jackzampolin
Copy link
Member Author

I think there is still some work that needs to be done here to rename the AccountMapper to the AccountKeeper on develop the file names are still mapper.go. I bet theres also some other spots that need to be updated.

@jackzampolin jackzampolin reopened this Oct 22, 2018
@alexanderbez
Copy link
Contributor

Was just about to say that 😆

@fedekunze
Copy link
Collaborator

I think this is mostly resolved, closing.

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

6 participants