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

Improve the structure of PostReaction #157

Closed
RiccardoM opened this issue May 11, 2020 · 2 comments · Fixed by #160
Closed

Improve the structure of PostReaction #157

RiccardoM opened this issue May 11, 2020 · 2 comments · Fixed by #160
Assignees
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add
Projects
Milestone

Comments

@RiccardoM
Copy link
Contributor

RiccardoM commented May 11, 2020

Context

Currently in order to represent a single post reaction, what we are doing is using this object:

// PostReaction is a struct of a user reaction to a post
type PostReaction struct {
  Owner sdk.AccAddress `json:"owner" yaml:"owner"` // Creator that has created the reaction
  Value string         `json:"value" yaml:"value"` // Value of the reaction, either an emoji or a shortcode
}

This is used also inside the GenesisData to represent all the reactions that have been added to the different posts.

Using this object from an outsider point of view, it is not enough in specific cases. Particularly, I've found some difficulties when handling it inside the genesis from within DJuno. Since the Value field can contain either the reaction shortocode or emoji value, it's hard to parse them and store them inside the database.

Solution

In order to solve the above mentioned problem I propose a revisit of this data type, in order to extend it so that it includes both the shortcode and the value of the emoji:

// PostReaction is a struct of a user reaction to a post
type PostReaction struct {
  Owner sdk.AccAddress `json:"owner" yaml:"owner"` // Creator that has created the reaction
  Shortcode string     `json:"shortcode" yaml:"shortcode"` // Shortcode of the reaction
  Value string         `json:"value" yaml:"value"` // Value of the reaction, either an emoji or an URI
}

Notes

I acknowledge that this approach comes with a con, which is the fact that emoji values and registered emojis will occupy a lot more storage than the current version due to the fact that their shortcode will be stored multiple times.

While this might be a problem, supposing we have all reactions with shortcodes that are 20 characters long (equivalent of 160 bytes, which is a very long shortcode to be honest), it would mean that in order to have a significant disk space change (1GB) we would have to have at least 6,250,000 reactions used. Personally I think that 1GB for 6 millions reactions is fair as long as that change allows for a better usage from the developer perspective.

@RiccardoM RiccardoM added the kind/enhancement Enhance an already existing feature; no "New feature" to add label May 11, 2020
@RiccardoM RiccardoM added this to the v0.6.0 milestone May 11, 2020
@RiccardoM RiccardoM changed the title Improve the structure of PostReaction Improve the structure of PostReaction May 11, 2020
@leobragaz
Copy link
Contributor

The change you suggest is good and I think that, if we do this to help for a better integration with middle-layers like Djuno, 1GB will not change our lives.
Also, these changes would not be so problematic for the whole codebase.

@kwunyeung
Copy link
Contributor

It's already another story for Desmos when there are 6m reactions 😄. Performance and data integrity is at higher priority than storage.

@RiccardoM RiccardoM added this to Done in Desmos via automation May 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add
Projects
No open projects
Desmos
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants