Beefy Boxes and Bandwidth Generously Provided by pair Networks
"be consistent"
 
PerlMonks  

Re: A small Deity AI class system

by jahero (Pilgrim)
on Nov 06, 2017 at 08:14 UTC ( [id://1202802]=note: print w/replies, xml ) Need Help??


in reply to A small Deity AI class system

Hi there!

Almost too long to read without further explanation, however - let me provide brief feedback based on what I have seen in package HollyGameAI::RNG. Please note that I am writing this without knowledge of context.

Mistake in sub *rollDX*.

Global symbol "$dx" requires explicit package name (did you forget to declare "my $dx"?) at test.pl line 22
Did you perhaps mean something like this?
sub rollDX { rand(shift->{dx}); }

Class constructor is usually named *new*. In your code, you are using constructor named RNG, which of course works, however this name is not something other people reading your code would expect to be. I would recommend sticking with the name *new* (minus the asterisk of course).

sub new { # this was named RNG my $class = shift; my $self = { dx => 0 }; return bless $self, $class; }

The way how you pass parameters to your subs is broken.

Consider this code:
sub roll { my ($self, $dxx) = shift;
This leaves variable $dxx undefined. Did you mean this?
sub roll { my ($self, $dxx) = @_;

Rather complex logic in sub *roll* could be simplified.

Seemingly, the *given* statement in your code is not needed, your *rollDX* covers all scenarios which are defined in *rollD1*, *rollD3*, *rollD<whatever>* routines. However, it is possible that these are actual placeholders which will contain additional code... So it is possible this is actually valid.
Anyway, the *given* statement could be replaced by something more readable, for example:
package HollyGameAI::RNG; use strict; use warnings; use Carp; our $roll_dispatch = { 0 => sub {0}, 1 => \&rollD1, 3 => \&rollD3, 6 => \&rollD6, 10 => \&rollD10, 20 => \&rollD20, }; sub roll { my ($self, $dxx) = @_; $self->set($dxx); return 0 unless defined $roll_dispatch->{$dxx}; return $roll_dispatch->{$dxx}->(); }

Final tip - it seems you are serious in that your code will be object oriented. If that's the case, I would recommend investing some time and learning one of Perl object oriented frameworks. As for myself, I have been using Moo for some time, and never looked back. Moo

Don't give up!

Regards, Jan

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://1202802]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others avoiding work at the Monastery: (6)
As of 2024-03-29 12:49 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found