Project

General

Profile

Actions

Feature #18183

closed

make SecureRandom.choose public

Added by olleicua (Antha Auciello) almost 3 years ago. Updated 11 months ago.

Status:
Closed
Target version:
-
[ruby-core:105361]

Description

This issue https://bugs.ruby-lang.org/issues/10849
added SecureRandom.alphanumeric and also the private method choose.
choose was kept private because the method name wasn't the best name to represent the behavior.
I think if it was called random_string it would be very clear what it does.
I also think it should be aliased to choose as well to allow backwards compatibility for people bypassing the private method with send (e.g. https://www.thetopsites.net/article/58611103.shtml)

I'm planning to put together a pull request for this. Please let me know if there are any complications I'm not considering.


Related issues 4 (0 open4 closed)

Related to Ruby master - Feature #10849: Adding an alphanumeric function to SecureRandomClosedandrewcbutterfield@gmail.com (Andrew Butterfield)Actions
Related to Ruby master - Feature #18190: Split `Random::Formatter` from securerandomClosedActions
Has duplicate Ruby master - Feature #18817: SecureRandom::choose is not being exposedRejectedActions
Has duplicate Ruby master - Feature #19854: Make SecureRandom.choose publicClosedActions
Actions #2

Updated by nobu (Nobuyoshi Nakada) almost 3 years ago

  • Related to Feature #10849: Adding an alphanumeric function to SecureRandom added
Actions #3

Updated by nobu (Nobuyoshi Nakada) over 2 years ago

  • Related to Feature #18190: Split `Random::Formatter` from securerandom added

Updated by matz (Yukihiro Matsumoto) over 2 years ago

I agree with the proposal, but choice is not a good name for a public method.

Matz.

Actions #5

Updated by mame (Yusuke Endoh) about 2 years ago

  • Has duplicate Feature #18817: SecureRandom::choose is not being exposed added

Updated by matz (Yukihiro Matsumoto) about 2 years ago

  • Status changed from Open to Rejected

SecureRandom.choose(*args) is not intuitive from my POV. Use args.sample(random: SecureRandom).

Matz.

Updated by matz (Yukihiro Matsumoto) about 2 years ago

My bad. args.sample(random: SecureRandom) is not a replacement for SecureRandom.choose(*args,len).
But still, choose does not describe the behavior.

Matz.

Updated by mame (Yusuke Endoh) about 2 years ago

@matz (Yukihiro Matsumoto) The OP proposed random_string. What do you think about it?

Updated by matz (Yukihiro Matsumoto) about 2 years ago

I don't think SecureRandom.random_string(args, len) describes the behavior, neither.

Matz.

Updated by midnight (Sarun R) about 2 years ago

My bad for not searching the old topics.
In this module, (most of) the method name describes the output, not the procedure, not how to generate the output.

For example:
Method hex outputs a hexadecimal number.
Method uuid outputs a uuid.

The problem now is that what do we call this thing:
"5FHY5PXLT184GIVISCVESTMGO"
given that the characters come from a specific input set.

My naive self would call it a string, but it is not just any string; maybe, "string" with some adjective?

Otherwise, what I call straight from my business application is "code".
SecureRandom.code maybe?

Updated by austin (Austin Ziegler) about 2 years ago

One option would be to extend Random::Formatter#alphanumeric to have an optional "alphabet":

def alphanumeric(n=nil, alphabet: ALPHANUMERIC)
  n = 16 if n.nil?
  choose(alphabet, n)
end

Updated by midnight (Sarun R) about 2 years ago

In case we really can't agree on the name.
I reimplement the functionality as an independent class on a GitHub's Gist; without using non-public API and the send hack, of course.

https://gist.github.com/midnight-wonderer/8fec1c670bd07f26b9466010842d2421

Usage:

code_generator = RandomCode.new(choose_from: ('A'..'Z').to_a)
code_generator.call(10)

It would be better if the functionality is in the core, though.

Updated by mame (Yusuke Endoh) about 2 years ago

austin (Austin Ziegler) wrote in #note-11:

One option would be to extend Random::Formatter#alphanumeric to have an optional "alphabet":

This approach looks nice to me. If "numeric" sounds a bit weird here, how about introducing Random::Formatter#alphabet(n = 16, alphabet: [*'A'..'Z', *'a'..'z'])?

Updated by austin (Austin Ziegler) about 2 years ago

mame (Yusuke Endoh) wrote in #note-13:

austin (Austin Ziegler) wrote in #note-11:

One option would be to extend Random::Formatter#alphanumeric to have an optional "alphabet":

This approach looks nice to me. If "numeric" sounds a bit weird here, how about introducing Random::Formatter#alphabet(n = 16, alphabet: [*'A'..'Z', *'a'..'z'])?

I think that #alphanumeric might still be the better name as the alphabet could be [*'ABCDFGHKMNPQRTUVWXYZ'.split(''), *'0'..'9']. If that’s not preferred, there are two choices, I think:

  • Random::Formatter#alphanumeric(n = 16, set: …)
  • Random::Formatter#from_set(set, n = 16) (or Random::Formatter#from_set(n = 16, set: …))

I think that alphabet is still the right word here, as it’s sort of the "term of art" for this sort of thing, although set is probably a good name as well. In any case, there should be a recommendation that the alphabet or the set parameter for any of these be a constant frozen value and not a localized constructed value.

Updated by olleicua (Antha Auciello) about 2 years ago

  • Random::Formatter#from_set(set, n = 16) (or Random::Formatter#from_set(n = 16, set: …))

I like Random::Formatter#from_set or Random::Formatter#from_alphabet because from makes it clear what the purpose of the additional parameter is.

Random::Formatter#alphabet would seem a little confusing to me as it could suggest that a standard a-z alphabet of some sort is being used.

Updated by matz (Yukihiro Matsumoto) about 2 years ago

I prefer Random::Formatter#alphanumeric(n = 16, chars: …) to others. But alphabet: or set: are acceptable too.

Matz.

Updated by mame (Yusuke Endoh) over 1 year ago

  • Status changed from Rejected to Assigned
  • Assignee set to nobu (Nobuyoshi Nakada)
Actions #18

Updated by mame (Yusuke Endoh) 11 months ago

Actions #20

Updated by nobu (Nobuyoshi Nakada) 11 months ago

  • Status changed from Assigned to Closed

Applied in changeset git|7e5c662a6f2e8435f8103bc16185bed6759cc557.


[Feature #18183] Add chars: option to Random#alphanumeric

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0