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 x/profiles params structure #538

Closed
RiccardoM opened this issue Jul 6, 2021 · 2 comments · Fixed by #546
Closed

Improve x/profiles params structure #538

RiccardoM opened this issue Jul 6, 2021 · 2 comments · Fixed by #546
Assignees
Labels
kind/enhancement Enhance an already existing feature; no "New feature" to add x/profiles Module that allows to create and manage decentralized social profiles
Milestone

Comments

@RiccardoM
Copy link
Contributor

Context

Currently the x/profiles parameters are represented by the following structures:

type Params struct {
  NicknameParams NicknameParams
  DTagParams     DTagParams
  MaxBioLength   sdk.Int
}

type NicknameParams struct {
  MinNicknameLength sdk.Int
  MaxNicknameLength sdk.Int
}

type DTagParams struct {
  RegEx         string
  MinDTagLength sdk.Int
  MaxDTagLength sdk.Int
}

In my opinion there are a bunch of things that can be improved about these structures:

  1. There are many redundant suffixes (Params, Nickname, etc)
  2. The bio params are not inside a subset of their own

Implementation proposal

To improve the structure of the parameters I think we can re-defined them as follows:

type Params struct {
  Nickanme Nickname
  DTag     DTag
  Bio   Bio
}

type Nickname struct {
  MinLength sdk.Int
  MaxLength sdk.Int
}

type DTag struct {
  ValidityRegEx         string
  MinLength sdk.Int
  MaxLength sdk.Int
}

type Bio struct {
  MaxLenght sdk.Int
}
@RiccardoM RiccardoM added kind/enhancement Enhance an already existing feature; no "New feature" to add x/profiles Module that allows to create and manage decentralized social profiles labels Jul 6, 2021
@leobragaz leobragaz self-assigned this Jul 6, 2021
@leobragaz
Copy link
Contributor

leobragaz commented Jul 7, 2021

Even though I think these changes are necessaries, while implementing this I felt that these names are too generic and may be a bit confusing in the long run, especially when others will put their hands on the code.
I have think about changing them like this:

type Params struct {
  NicknameLimits NicknameLimits
  DTagPolicies DTagPolicies
  BioLimits BioLimits
}

type NicknameLimits struct {
  MinLength sdk.Int
  MaxLength sdk.Int
}

type DTagPolicies struct {
  ValidityRegEx string
  MinLength sdk.Int
  MaxLength sdk.Int
}

type BioLimits struct {
  MaxLength sdk.Int
}

let me know what you think @RiccardoM

@RiccardoM
Copy link
Contributor Author

RiccardoM commented Jul 8, 2021

@bragaz I agree these names are more detailed. Can we just use Nickname, DTag and Bio for the field names though? Since they are fields of the Params struct I think we can remove the Limits and Policies suffixes. Also I think we can then revert to the old struct naming. What bothered me were only the field names:

type Params struct {
  Nickname NicknameParams
  DTag DTagParams
  Bio BioParams
}

@RiccardoM RiccardoM added this to the v0.17.1 milestone Jul 9, 2021
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 x/profiles Module that allows to create and manage decentralized social profiles
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants