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

Rewriting this some recommendations #74

Open
wade-welles opened this issue Jan 29, 2022 · 2 comments
Open

Rewriting this some recommendations #74

wade-welles opened this issue Jan 29, 2022 · 2 comments

Comments

@wade-welles
Copy link

You can replace your misnamed RandomInt32 (which should be RandomUint32 because it returns a uint32) with a single return rand.Uint32()

Most of the bitwise mess that is intelligible to non-C developers, and often makes them cry when they look at it with binary.(Big|Little)Endian.PutUint(16|32)

I also found a way to shrink the timestamp data to 2 bytes while keeping the most important aspects of it.

If there is any interest I can make pull requests with some of these changes to make it more readable to most Go programmers and make it easier to modify and update because right now I imagine most developers who don't know bitwise operataions very well look at this codebase and cry and its not making it more efficient.

There is tons of efficiency issues in the code, it uses way way more memory than it needs to.

@wade-welles
Copy link
Author

Also unless you are getting incredible speed increase from your custom implementation of base32 encoding. Almost half of your code-base could be replaced with

     encoder := base32.NewEncoding("0123456789abcdefghijklmnopqrstuv").WithPadding(base32.NoPadding)
     base32Id := encoder.EncodeToString(id)
     fmt.Println("custom encoder:", base32Id)

This is not just smaller, its easier to understand, and therefore easier for other developers to interact with. Personally I don't have a problem reading the codebase but I know from experience that many developers would have trouble, and some may cry.

You can see these and other refactoring I did (to the point of being a different library since I merged in bsonid too and added several more features including checksums) @ github.com/multiverse-os/muid

If you are interested in refactoring this library let me know I would be interested in contributing.

@rs
Copy link
Owner

rs commented Jan 29, 2022

Ok to rename the misnamed randInt. The rest is about avoiding allocations and bound checks. If you think you can make the code simpler while not affecting the performance, go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants