randomatic icon indicating copy to clipboard operation
randomatic copied to clipboard

Should it throw an error on `randomatic('?', 4)`?

Open benfletcher opened this issue 6 years ago • 1 comments

randomatic('?', 4)

Currently if the chars option is not specified, the characters from undefined are used as the custom characters (including the fact that d, n and e are 2x more frequent than the remaining letters.

Should an error be thrown instead? Or maybe [a-z] used and a message logged to that effect?

The readme does indeed specify that the default is undefined, but it was still a little surprising to me that it was allowed to be coerced to a string if not user-specified.

benfletcher avatar Feb 19 '19 15:02 benfletcher

I think this is a bug in that opts.chars should be checked before doing mask += opts.chars.

Also, the readme does say specify to pass in the option when using ?:

?: Custom characters (pass a string of custom characters to the options)

I think we either throw an error or change this line to if (pattern.indexOf('?') !== -1) mask += (opts.chars || '');

I prefer the latter since it will fix the "bug" of undefined being used, but not change the API (e.g. throwing a new error) so we can do a patch bump.

A PR is welcome to make the change and update the .verb.md file with some information about what happens when ? is used and opts.chars is not set.

doowb avatar Feb 19 '19 16:02 doowb