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

Add methods to convert it to ULID string for only the time part and random part #29

Closed
wants to merge 1 commit into from

Conversation

moznion
Copy link
Contributor

@moznion moznion commented Aug 18, 2023

This pull request introduces the following 4 methods in Ulid class:

  • #toTimePartUppercaseString()
  • #toTimePartLowercaseString()
  • #toRandomPartUppercaseString()
  • #toRandomPartLowercaseString()

These methods convert it to ULID string for only the time or random part.

Motivation

For example, when the users want to expand the ULID format on their own project based on this library, e.g. inserting the monotonic sequence number between a time part and a random part, we used to have to use String#substring() for the generated string. This pull request aims to make it easy to do by providing the ULID string for time and random parts individually.

@fabiolimace
Copy link
Member

fabiolimace commented Aug 27, 2023

Apologies for the delay!

Unfortunately, I can't add the new methods because they handle a very specific case that can be solved with the String#substring() method. In addition, the change adds a concatenation to the Ulid#toString() method, which can slow down this method for the vast majority of cases that only need this method.

If you want to improve string insertion performance, you can use a StringBuilder instead of concatenation. I did a small benchmark that shows that StringBuilder is almost 3x faster than concatenation. Of course this is just a suggestion.

    public static String insertUsingStringBuilder(String string, String insert) {
	    final int position = 10;
	    StringBuilder builder = new StringBuilder(string);
	    builder.insert(position, insert);
	    return builder.toString();
    }

Anyway, I really appreciate your work!

@moznion moznion deleted the part_string_getter branch August 27, 2023 22:08
@moznion
Copy link
Contributor Author

moznion commented Aug 27, 2023

Thank you for your feedback.
Yes, the performance of the substring method is a reason why I sent this patch, but as you suggested, using StringBuilder seems nice for that. Thank you!

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

Successfully merging this pull request may close these issues.

2 participants