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

Consider introducing a type alias for NamespaceVersion #35

Open
rootulp opened this issue Mar 30, 2023 · 5 comments
Open

Consider introducing a type alias for NamespaceVersion #35

rootulp opened this issue Mar 30, 2023 · 5 comments
Labels
good first issue Good for newcomers

Comments

@rootulp
Copy link
Collaborator

rootulp commented Mar 30, 2023

[optional] What do you think about type aliasing here? i.e., defining a distinct type for the NamespaceVersion, this will facilitate future refactoring and debugging

Originally posted by @staheri14 in celestiaorg/celestia-app#1529 (comment)

@rootulp
Copy link
Collaborator Author

rootulp commented Mar 30, 2023

FWIW the same approach can be adopted by ShareVersion

@rootulp rootulp added good first issue Good for newcomers and removed needs:triage labels Mar 30, 2023
@staheri14
Copy link
Contributor

staheri14 commented Mar 31, 2023

I believe that type aliasing is important for several reasons. Firstly, it can greatly aid in the process of refactoring code (discussed before). Also, if we define a variable as uint8, it may appear arbitrary and not be immediately clear how it fits into the overall protocol-level decision. As the codebase grows and more variables of type uint8 are defined, it can become increasingly difficult to keep track of which ones correspond to a specific type, such as NamespaceVersion or ShareVersion.

Secondly, type aliasing allows us to implement helper functions that are specific to a particular type, such as a validity check for the NamespaceVersion type (we may also need to define custom math operations over them, like comparison, subtraction, etc.). By contrast, if we were using uint8, it would be much more difficult to ensure that the value being checked corresponds to the intended meaning. For example, uint8 could represent an index, a temperature, a time value, or any number of other things. Type aliasing helps to clarify the semantic of the data type and disallows misuses, making the code more robust and easier to maintain over time.

@NishantKoyalwar
Copy link

hii @staheri14 @rootulp , i am interested on working on this issue but i think we should use type declaration type NamespaceVersion uint8 (exactly like enum are used in golang) instead of type aliases (type abc = xyz) they usually serve purpose of depricating types or helpful in refactoring

let me know what do you think of this and i will create a new pr

@staheri14
Copy link
Contributor

Using this form type NamespaceVersion uint8 is exactly what I meant by aliasing. 👍

@Wondertan
Copy link
Member

A quick note that aliasing in Golang is slightly different from new type definition. https://yourbasic.org/golang/type-alias/

Might worth reconsidering which of the approaches suites the needs better

@rootulp rootulp transferred this issue from celestiaorg/celestia-app Feb 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

4 participants