Beefy Boxes and Bandwidth Generously Provided by pair Networks
P is for Practical
 
PerlMonks  

RFC: Proofread POD for my main random module, please? (more)

by Lady_Aleena (Curate)
on Jun 01, 2017 at 19:31 UTC ( #1191887=perlquestion: print w/replies, xml ) Need Help??
Lady_Aleena has asked for the wisdom of the Perl Monks concerning the following question:

Hello. Would some gentle monks please proofread my POD for my main random module and let me know if it makes sense?

Also, I would love some help renaming the module. It is currently sitting at RolePlaying::Random. I am moving most of my random generation modules to Random::. If one day I decide to share my randomness with CPAN, I do not feel I can take the top spot in the Random name space for this module. It would not be right unless I get a whole lot of people from here and CPAN telling me it is okay. I am leaning toward Random::util, but I am not sure of it. What do you name modules that you use to build other modules?

Please let me know if you see something in the code that needs improvement, too.

Thank you in advence!

Update 1: From the responses, I made changes that can be found here.

Update 2: From the additional responses, I made more changes that can be found here.

package Random::???; use strict; use warnings FATAL => qw(all); use Exporter qw(import); our @EXPORT_OK = qw(random tiny_rand instant_rand); use List::Util qw(shuffle); sub random { my ($list, $user_input, $opt) = @_; my $random_thing; if ($user_input && $user_input =~ /(?:help|options)/) { my $keys = join("\n\t", sort keys %{$list}); $random_thing = "Your options are: $keys 'by keys' to get a random key 'data' to get the whole hash 'keys' to get a list of the keys 'all' to get a random item from any key on the list"; } elsif ($user_input && $user_input eq 'data') { $random_thing = $list; } elsif ($user_input && $user_input eq 'keys') { $random_thing = [keys %$list]; } else { my @random_list; if ($user_input && $user_input eq 'by keys') { @random_list = keys %{$list}; } elsif (!$user_input || $user_input eq 'all' ) { @random_list = map { @$_ } values %{$list}; } elsif ($list->{$user_input}) { @random_list = @{$list->{$user_input}}; } else { my $caller = $opt->{caller} ? " from ".$opt->{caller} : ''; die "Your option $user_input does not produce a list$caller.\n\t +Stopped$!" } push @random_list, @{$opt->{'additions'}} if $opt->{'additions'}; @random_list = shuffle(@random_list); $random_thing = $random_list[rand @random_list]; } return $random_thing; } sub tiny_rand { my ($var1, $var2) = @_; return ($var1, $var2)[rand 2]; } sub instant_rand { my @rand_array = @_; return $rand_array[rand @rand_array]; } =head1 NAME B<Random::???> is a set of tools to generate randomness. =head1 SYNOPSIS use Random::??? qw(random tiny_rand instant_rand); my $random_thing = random($hash_of_arrays, $list, $options); my $tiny_rand = tiny_rand(qw(black white)); my $instant_rand = instant_rand(qw(red yellow green cyan blue magent +a)); =head1 DESCRIPTION B<Random> has three tools to create randomness: C<random>, C<tiny_rand +>, and C<instant_rand>. =head2 random C<random> takes a hash of arrays and returns a random result based on +the list selected from the hash. It is meant to be used in the creati +on of other random subroutines. sub random_color { my ($color_key, $color_additions) = @_; random($color_hash, $color_key, { caller => 'random_color', additi +ons => $color_additions}); } =head3 data When C<data> is selected, C<random> will return the entire hash so you + can refresh your memory of what is in the hash. =head3 keys When C<keys> is selected, C<random> will return the list of keys from +the hash to help remind you of your key options. =head3 by keys When C<by keys> is selected, C<random> will return a random key. =head3 a key When a specific key is selected, C<random> will return an item from th +e selected key. =head3 all or nothing When C<all> or C<undef> is selected or nothing is entered, C<random> w +ill flatten the hash and return any value from the hash. =head3 options There are two options you can use, C<additions> and C<caller>. C<additions> is a list (arrayref) that you want to add to the list you + have already chosen. C<caller> can be used to create an error message with the specific sub +routine your are using random in. From the example above, if the user chooses a list that will not produ +ce a result, the error message will read as follows. Your option list does not produce a list from random_color. Stopped at ... =head2 tiny_rand C<tiny_rand> takes an array of two items and returns a random result. =head2 instant_rand C<instant_rand> takes an array of any size and returns a random result +. =head1 AUTHOR Lady Aleena =cut 1;

I did not realize until I posted this how long the pod got.

No matter how hysterical I get, my problems are not time sensitive. So, relax, have a cookie, and a very nice day!
Lady Aleena

Replies are listed 'Best First'.
Re: RFC: Proofread POD for my main random module, please? (more)
by hippo (Abbot) on Jun 01, 2017 at 22:04 UTC

    In your synopsis you have this line:

    my $random_thing = random($hash_of_arrays, $list, $options);

    but in reading the source it appears that the second argument is expected to be a scalar, not a list. That's confusing.

    Also you say several times, "When XXX is selected" but give no clue as to what "selected" means in this context. Again, it is only by reading the source that we can see you mean "passed as a literal value as the second argument to random()".

    I think it would help a great deal if in your snippet listing the example random_color() sub you were actually to specify values for the three variables instead of leaving us all to wonder what they might be (or even what form they should take). eg.

    my $key = 'foo'; my $additions = 'elephant'; my $color_hash = { a => [qw/ I do not know /], b => [qw/ what should + go here /] }; my $rv = random_color ($key, $additions); sub random_color { my ($color_key, $color_additions) = @_; random($color_hash, $color_key, { caller => 'random_color', additi +ons => $color_additions}); }

    Finally, I don't see why instant_rand() is so named. Why not list_item_rand()?

    HTH.

Re: RFC: Proofread POD for my main random module, please? (more)
by pryrt (Curate) on Jun 01, 2017 at 22:16 UTC
    • $color_hash magically appears in your random_color() example usage, and you give no indication of where it comes from (it's not a parameter to your example sub) or what it should look like (other than an arbitrary "hash(ref) of arrays")
    • It takes multiple readings of the POD for me to guess that the data, keys, by keys, ... under the random() function refer to string values of the $color_key argument. Make sure you put quotes around each of the strings, but not around the undef.
    • For tiny_rand(), you might want to mention "a coin flip", and give an example, like tiny_rand("HEADS", "TAILS");. The name is not helpful; maybe rename (or alias) to random_from_two()
    • For instant_rand(), give an example, like instant_rand("HEADS", "TAILS", "EDGE", "EMBED IN CEILING");. Maybe rename (or alias) to random_from_list().
Re: RFC: Proofread POD for my main random module, please? (more)
by Marshall (Abbot) on Jun 01, 2017 at 21:49 UTC
    Hi Lady_Aleena,

    I started by downloading your .pl code and then generating the HTML. I only looked at the HTML doc, not your code in this review. Looking at the code would perhaps solve lots of questions, but that is not the point of a doc review.

    I don't understand the main purpose of this module other than it has something to do with randomizing things. Randomize what? And why? How does your module do much more than obvious uses of the standard Perl rand functions?

    Why is there a separate tiny_rand() from instant_rand()? Why the need for 2 functions? As for naming why wouldn't a "tiny" one be "instant"?

    As a naive person to your topic, I just do not understand what this thing is supposed to do.
    I am not trying to "rain on your parade". But, I think the doc's could be improved. superfluous comment deleted.

    update: Starting to look at the actual code and I agree with Re: RFC: Proofread POD for my main random module, please? (more) from pyrt in regards to "tiny_rand()" and "instant_rand()". I would think random_from_list() as pyrt suggests is more understandable. I don't see a reason to expose the 2 parameter version of "instant_rand()" as "tiny_rand()" unless there is some significant performance reason to do so.

Re: RFC: Proofread POD for my main random module, please? (more)
by Lady_Aleena (Curate) on Jun 02, 2017 at 04:40 UTC

    Changes

    • Broadened examples.
    • Example usage with each section of POD.
    • tiny_rand and instant_rand now aliases for random_from_list (name suggested by pryrt and seconded by Marshall, though hippo suggested another name).
    • Current name restored, though I am still interested in help on renaming it.

    About the names tiny_rand and instant_rand and why tiny_rand wasn't just expanded when I came up with instant_rand, they seemed like good idea at the time. On reflection, they are not. So your suggestions were taken and used. However, random_from_list doesn't flow out of the fingertips as quickly as tiny_rand does.

    pryrt, when you said don't quote undef, please know I didn't "quote" anything. I used the C<> POD markup around the words that are quoted in perldoc view. So C<undef> becomes "undef" in the perldoc viewer. In CPAN's POD checker there are no quotes unless I typed them into the POD.

    I hope the expanded examples help, though the POD is now so very very long for such short code.

    No matter how hysterical I get, my problems are not time sensitive. So, relax, have a cookie, and a very nice day!
    Lady Aleena

      Personally, I think that this is a great improvement over the original and that the examples add much to the clarity of purpose of the module as a whole. Well done.

      Now that it's clear (to me) what the module's purpose is, I would suggest changing the nameline from "RolePlaying::Random is a set of tools to generate randomness." to something like "Randomly select values from sets of lists.". I think that this more accurately reflects what the module does. It certainly doesn't generate randomness, that's for a PRNG to perform. What it does is take randomness generated by other code and apply it in various ways to supplied datasets.

      random_from_list doesn't flow out of the fingertips as quickly as tiny_rand does.

      Maybe not. This helps to illustrate one of the differences between coding for yourself and coding for others. In the latter case, clarity wins every time. Naming your subs (and other symbols such as packages, variables and constants) to follow their purpose helps make the code which others write using them to be legible and maintainable. If you (or anyone) really wants then in their own code to use a shorthand for a sub they can by aliasing or subclassing or whatever. But careful choice of symbol names improves the chances of code being self-documenting and that's a good target to aim at.

      FWIW, I seen nothing in this code which ties it to the RolePlaying space. It seems perfectly general. I've just looked around the namespaces and there's neither List::Random nor Random::List. There is, however, Random::Set which seems to perform a similar task but without your bells and whistles. If you think that Random::Set is no more general than your module then maybe consider a more general name for yours. Don't forget PrePAN for soliciting ideas too.

      ++ Much improved. My point with mentioning "quotes" was I was hoping When C<all> or C<undef> is selected would be When C<'all'> or C<undef> is selected (and similarly for other textual literals, like C<'by keys'> or C<'keys'>). But with the examples that now follow, that's quickly clarified, so it's not critical.

      nitpick #1: random_from_list should be included in @EXPORT_OK

      nitpick #2: in the =head4 keys section, there are typos of "randon" and "randon_color"

      I agree with hippo that List::Random and Random::List both seem to apply as reasonable names. Given Random::Set and Random::Day, which both select a random element from a group, I think I'd have a slight preference for Random::List, because your module complements those nicely. (I agree with your OP, in that taking the top spot of package Random; is not appropriate.) I also ++agree with hippo's Synopsis suggestion.

      Don't worry about long POD: in my opinion, long-but-helpful is much better than short-and-unhelpful. As a new user to a module, I want it descriptive enough that someone who doesn't have the author's history with the module would be able to read the documentation and immediately grasp what the module is about and how to use it.

      Changes...

      • Relocated module from RolePlaying name space to Fancy name space.
      • Renamed
        • module from Random to Rand
        • random to fance_rand
        • random_from_list to fancy_rand_from_array (a hash is also a list)
      • Corrected errors seen by others.
      • Added uniq to the flattened hash list so repeated items don't get more weight.

      You might wonder why Fancy::Rand. Well, I already have Fancy::Map, Fancy::Splice, Fancy::Split, Fancy::Join::Defined, and Fancy::Join::Grammatical. I decided when I do something "fancy" with a perl function, I will put it in the name space Fancy and name the top subroutine fancy_function. So, now I can add Fancy::Rand and fancy_rand to my list of Fancy::Functions.

      Here is the final rough draft. I think I got everything fixed.

      package Fancy::Rand; use strict; use warnings FATAL => qw(all); use Exporter qw(import); our @EXPORT_OK = qw(fancy_rand fancy_rand_from_array tiny_rand instant +_rand); use List::Util qw(shuffle uniq); sub fancy_rand { my ($list, $user_input, $opt) = @_; my $random_item; if ($user_input && $user_input =~ /(?:help|options)/) { my $keys = join("\n ", sort keys %{$list}); $random_item = "Your options are: $keys by keys: get a random key all: get a random item from any key on the list keys: get the list of the keys data: get the whole hash"; } elsif ($user_input && $user_input eq 'data') { $random_item = $list; } elsif ($user_input && $user_input eq 'keys') { $random_item = [keys %$list]; } else { my @random_list; if ($user_input && $user_input eq 'by keys') { @random_list = keys %{$list}; } elsif (!$user_input || $user_input eq 'all' ) { @random_list = uniq(map { @$_ } values %{$list}); } elsif ($list->{$user_input}) { @random_list = @{$list->{$user_input}}; } else { my $caller = $opt->{caller} ? " in ".$opt->{caller} : 'fancy_ran +d'; die "Your option '$user_input' is not a list $caller.\n\tStopped +$!" } push @random_list, @{$opt->{'additions'}} if $opt->{'additions'}; @random_list = shuffle(@random_list); $random_item = $random_list[rand @random_list]; } return $random_item; } sub fancy_rand_from_array { my @rand_array = shuffle(@_); return $rand_array[rand @rand_array]; } sub tiny_rand { fancy_rand_from_array(@_); } sub instant_rand { fancy_rand_from_array(@_); } =head1 NAME B<Fancy::Rand> selects random items from sets of lists. =head1 SYNOPSIS use Fancy::Rand qw(fancy_rand fancy_rand_from_array tiny_rand instan +t_rand); my $random_thing = fancy_rand($hash_of_arrays, $selected_list, { add +itions => [@additional_items], caller => $caller }); my $fancy_rand_from_array = fancy_rand_from_array(qw(red yellow gree +n cyan blue magenta black white)); =head1 DESCRIPTION B<Fancy::Rand> has two tools to help you select where your randomness +comes from: C<fancy_rand> and C<fancy_rand_from_array>. =head2 fancy_rand C<fancy_rand> takes a hash of arrays and returns a random result based + on the list selected from the hash. It is meant to be used in the cr +eation of other random subroutines. my %colors = ( 'general' => [qw(red yellow green cyan blue magenta white black gr +ay)] 'eye' => [qw(amber black blue brown gray green hazel red violet)] +, 'hair' => [qw(auburn brown black blond gray red white)], 'rainbow' => [qw(red orange yellow green blue indigo violet)], ); sub random_color { my ($selected_color_key, $color_additions) = @_; fancy_rand(\%colors, $selected_color_key, { additions => $color_a +dditions, caller => 'random_color' }); } =head3 Selections =head4 all or nothing When C<all> or C<undef> is selected or nothing is entered, C<random> w +ill flatten the hash and return any value from the hash. Using C<fancy_rand> by itself: my $random_color_one = fancy_rand(\%colors); my $random_color_two = fancy_rand(\%colors, undef); my $random_color_all = fancy_rand(\%colors, 'all'); Using the newly created C<random_color>: my $random_color_one = random_color(); my $random_color_two = random_color(undef); my $random_color_all = random_color('all'); All of the above will return any color in the hair, eye, and rainbow l +ists. =head4 a key When a specific key is selected, C<fancy_rand> will return an item fro +m the selected key. Using C<fancy_rand> by itself: my $random_hair_color = fancy_rand(\%colors, 'hair'); my $random_eyes_color = fancy_rand(\%colors, 'eye'); Using the newly created C<random_color>: my $random_hair_color = random_color('hair'); my $random_eyes_color = random_color('eye'); =head4 by keys When C<by keys> is selected, C<fancy_rand> will return a random key. Using C<fancy_rand> by itself: my $random_color_key = fancy_rand(\%colors, 'by keys'); Using the newly created C<random_color>: my $random_color_key = random_color('by keys'); Both of the above will return a random key from C<%colors>: hair, eye, + rainbow. =head4 keys When C<keys> is selected, C<fancy_rand> will return the list of keys f +rom the hash to help remind you of your key options. Using C<fancy_rand> by itself: my $random_color_keys = fancy_rand(\%colors, 'keys'); Using the newly created C<random_color>: my $random_color_keys = random_color('keys'); Both of the above will return a list of the keys: C<['hair', 'eye', 'r +ainbow']>. =head4 data When C<data> is selected, C<fancy_rand> will return the entire hash so + you can refresh your memory of what is in the hash. Using C<fancy_rand> by itself: my $random_color_data = fancy_rand(\%colors, 'data'); Using the newly created C<random_color>: my $random_color_data = random_color('data'); =head4 help or options When C<help> or C<options> is selected, all of your options will be li +sted. Using C<fancy_rand> by itself: my $random_color_help = fancy_rand(\%colors, 'help'); my $random_color_opts = fancy_rand(\%colors, 'options'); Using the newly created C<random_color>: my $random_color_help = random_color('help'); my $random_color_opts = random_color('options'); =head3 Optional parameters There are two optional parameters you can use, C<additions> and C<call +er>. =head4 additions C<additions> is a list that you want to add to the list you have alrea +dy selected. You might want to add the colors pink, blue, and purple to the choices + of hair color or yellow and bloodshot to eye color. Using C<fancy_rand> by itself: my $random_hair_color = fancy_rand(\%colors, 'hair', { additions => +[qw(pink purple blue)] }); my $random_eyes_color = fancy_rand(\%colors, 'eye', { additions => +[qw(yellow bloodshot)] }); Using the newly created C<random_color>: my $random_hair_color = random_color('hair', [qw(pink purple blue)]) +; my $random_eyes_color = random_color('eye', [qw(yellow bloodshot)]) +; =head4 caller C<caller> can be used to create an error message with the specific sub +routine your are using random in. Using C<fancy_rand> by itself: my $random_color = fancy_rand(\%colors, 'rainboe', { caller => 'rand +om_color' }); Using the newly created C<random_color>: my $random_color = random_color('rainboe'); If the user selects a list that will not produce a result, the error m +essage from both of the above will read as follows. Your option 'rainboe' is not a list in random_color. Stopped at ... =head2 fancy_rand_from_array C<fancy_rand_from_array> takes an array and returns a random result. C +<tiny_rand> and C<instant_rand> are aliases for C<fancy_rand_from_arr +ay>. my $fancy_rand_from_array = fancy_rand_from_array(qw(black white red + yellow green cyan blue magenta)); my $tiny_rand = tiny_rand(qw(black white)); my $instant_rand = instant_rand(qw(red yellow green cyan blue magent +a)); =head1 AUTHOR Lady Aleena =cut 1;
      No matter how hysterical I get, my problems are not time sensitive. So, relax, have a cookie, and a very nice day!
      Lady Aleena

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://1191887]
Approved by Discipulus
help
Chatterbox?
[erix]: installing perfect pitch?
[karlgoethebier]: ...good Hot tuna. Note the stacks & racks in the background
[karlgoethebier]: ..the last hippies. They still perform. At least the survivors...
[Discipulus]: not survived
[Discipulus]: has pm some danish monk?
[Discipulus]: erix the problem was not rain in the brain.. ;=)

How do I use this? | Other CB clients
Other Users?
Others imbibing at the Monastery: (8)
As of 2017-11-17 20:20 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?
    In order to be able to say "I know Perl", you must have:













    Results (272 votes). Check out past polls.

    Notices?