My code generates 9 random characters, grouping them into 3 letter groups from an array of permissible characters in the form:
A4b-c56-7de
Ie, 9 upper and lower letters/numbers, separated in groups of 3 with a '-' in between. However, since I need this for other generic stuff later on, the function accepts as arguments, the number of letters (eg 9) and the number in the group (eg 3).
function CreateMyRandomId(numLetters, numInGroup){
// Returns a string
}
I have used array manipulating functions for the first time.
- I use the Comma Operator so I can have a single line of code to make use of the auto return.
- I chain the array functions.
- I generate my permissible characters by array functions and the use of base 36 in toString.
I am reasonably happy, but not quite. I would also like any constructive criticism, so I can become better.
I generate an array of the permissible characters, excluding 'o', 'O' and '0'. This code will need to be typed in by the user, so characters need to be easy to differentiate from each other.
Here is how I generate my array of permissible characters:
// [0-9a-zA-Z] but excluding o, 0 and O that look the same
var myChars = [... Array(10+2*26)]
.map((_, idx) => {
var bl, ret;
return (bl = idx >= 36, ret = (idx - (bl ? 26 : 0)).toString(36), bl && (ret = ret.toUpperCase()), ret);
})
.filter(val => !["o","O","0"].includes(val));
I do not like the map
here. I wanted the auto-return, ie where there is only one statement, so you do not need to write the return. I tried to use the comma operator where The last thing will be the thing returned. However, you cannot appear to declare variables, bl
and ret
within the comma operator. That means you have to write a second line, which means you do not get the auto return.
You can obviously introduce a second map into the code to do the upper case. Is this good, is this bad? As a hobby programmer, I do not have anyone to say which is better.
I could do:
// [0-9a-zA-Z] but excluding o, 0 and O that look the same
var myChars = [... Array(10+2*26)]
.map((_, idx) => (idx - (idx >= 36 ? 26 : 0)).toString(36))
.map((val, idx) => idx >= 36 ? val.toUpperCase() : val)
.filter(val => !["o","O","0"].includes(val));
This obviously looks a million times nicer and is easy for a third-party to read and understand. But is it as fast, is it better code?
As for the random string generator, I found that a lot easier to write, again using the comma operator for the auto return:
function CreateMyRandomId(numLetters, numInGroup) {
return [... Array(numLetters)]
.map(() => myChars[(Math.random() * myChars.length) | 0])
.reduce((endArr, curVal, curIdx /*, arr not needed*/) =>
// Left most thing in (, ) is what is returned
(endArr[(curIdx / numInGroup) | 0].push(curVal), endArr),
//[... Array(Math.ceil(numLetters / numInGroup))].fill([]))
[... Array(Math.ceil(numLetters / numInGroup))].map(() => []))
.map(curMiniArr => curMiniArr.join(''))
.join('-');
}
I want an alternative to Math.ceil
.
Math.floor
can be replicated for (max bitwise safe range) floats by doing myFloat | 0
.
There must be something that can be done for ceil. (I have had an idea, maybe -(~myFloat)
, or something on those lines. I am going to try to think of something later.
I had to create the mini arrays holding 3 letters each. I set those up at the start, and have left my mistake in (hashed out to remind me later). When I did [... Array(Math.ceil(numLetters / numInGroup))].fill([])
that used the same array for each of the mini arrays, which was wrong. I had to do [... Array(Math.ceil(numLetters / numInGroup))].map(() => [])
instead to get unique mini arrays.
The function is run with:
var str = CreateMyRandomId(9, 3);
Again, any criticism of this would be much appreciated. I want to get better. I aspire to be as good as all the people on here who answer these type of questions. I aspire to be a guy who answers these questions for others.