refactor: simplified genRndStr6 without recursion #1107
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the genRndStr6 function to eliminate recursion by using padEnd to ensure the returned string is always 6 characters long, padding with '0' when necessary.
- Simplified the random string generation logic by removing conditional recursion
- Used
padEndmethod to guarantee 6-character string length with '0' padding
| const str = (Math.random() + 1).toString(36).substring(2, 8); | ||
| if (str.length === 6) return str; | ||
| else return genRndStr6(); | ||
| return Math.random().toString(36).slice(2, 8).padEnd(6, '0'); |
There was a problem hiding this comment.
The refactored implementation changes the behavior from the original. Math.random().toString(36) can produce strings shorter than 8 characters (including the '0.' prefix), so slice(2, 8) may return fewer than 6 characters. While padEnd addresses the length issue, it introduces '0' characters that weren't present in the original recursive approach, potentially affecting uniqueness and the expected character distribution.
| return Math.random().toString(36).slice(2, 8).padEnd(6, '0'); | |
| let str = ''; | |
| while (str.length < 6) { | |
| str += Math.random().toString(36).slice(2); | |
| } | |
| return str.slice(0, 6); |
streamich
left a comment
streamich
left a comment
There was a problem hiding this comment.
LGTM. Thank you for the Gist.
|
🎉 This PR is included in version 4.18.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
2 participants
I slightly simplified the
genRndStr6function by usingpadEndfor cases where the string is shorter than 6 characters. In most cases, padEnd isn’t applied, and the strings are sufficiently unique—just like in the original version.https://gist.github.com/Connormiha/c42e57ecd1f934dea49877f5d7f815cb
By the way, package.json specifies Node >=4, but I believe that's outdated, since the project already uses
includes,await, and the spread operator.