Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl Monk, Perl Meditation
 
PerlMonks  

Re: RFC: Creating Dancer2 validation plugin.

by stevieb (Canon)
on May 04, 2022 at 15:32 UTC ( [id://11143576]=note: print w/replies, xml ) Need Help??


in reply to RFC: Creating Dancer2 validation plugin.

I've got just a couple of comments after a cursory look...

In tests, pieces of code are often repeated, is there a generally accepted method of reusing them?

Depending on what those pieces are, put them into a library (say, TestLib.pm in the t/ directory). Then:

use warnings; use strict; use lib 't/'; use TestLib; use Test::More; ...

It becomes a pain in the rear to have to modify several tests in several files if you change the code tests are testing.

The documentation appears to be very thorough. Good job. I've passed up on what look like half decent Dancer2 plugins due to lack of documentation.

Also kudos on using Email::Valid instead of regex for email validation. Realistically, to use regex to thoroughly validate an email address, you'd be dealing with a regex straight from the depths of hell.

About the code (I took a glance, I didn't go through it thoroughly)... I personally like to see use strict; and use warnings; at the top of every Perl file, script or library. Also, it's best practice to assign all parameters from the @_ array at the top of a function/method. For example, instead of:

sub _register_hooks { my $self = shift; $self->app->add_hook( Dancer2::Core::Hook->new( name => 'before_template_render', code => sub { my $tokens = shift; ...

I'd have:

sub _register_hooks { my ($self, $token) = @_; ...

This way, anybody reading your code understands exactly how many and which parameters the sub wants and/or needs. To further, if you ever change the parameters coming in, you don't have to go searching for their order by trying to seek out shift statements half way through the sub. Doing it by shifting so far into the sub can lead to head scratching as to why you're getting 'baz' as a parameter for $token instead of token data, because you shifted off a new parameter that wasn't aligned properly.

Using my ($x, $x, %z) = @_; is essentially self-documenting code, on the first line of the sub definition.

For small things like this:

sub BUILD { shift->_register_hooks; return; }

I prefer to write it like:

sub BUILD { $_[0]->_register_hooks; }

Again, just a preference, but it leaves @_ intact, and in a method, $_[0] unambiguously refers to the object (or class, as it were).

Same as below. If my sub grows to three or more lines, or has more than two parameters, I replace the $_[0] etc with the way I showed above.

sub _validator_language { shift->config_validator->language(shift); return; }

...

sub _validator_language { $_[0]->config_validator->language($_[1]); }

...is there a reason you're intentionally returning undef?

As far as private methods, because I don't often document them (because they're usually very small and specific), I do usually add a small comment before assigning parameters just so it's easy for others to see what it's to be doing.

sub _access_token_data { # Fetches and stores the access token data to the AUTH_CACHE_FILE my ($self, $data) = @_; ... } sub _access_token_set_expiry { # Sets the access token expiry date/time after generation and # renewal my ($self, $token_data) = @_; }

Replies are listed 'Best First'.
Re^2: RFC: Creating Dancer2 validation plugin.
by choroba (Cardinal) on May 05, 2022 at 08:51 UTC
    >
    sub _register_hooks { my $self = shift; $self->app->add_hook( Dancer2::Core::Hook->new( name => 'before_template_render', code => sub { my $tokens = shift; ...
    I'd have:
    sub _register_hooks { my ($self, $token) = @_; ...

    But that's not equivalent! Note the anonymous sub, the second shift happens in a nested subroutine, it uses its arguments, not the _register_hooks' ones.

    map{substr$_->[0],$_->[1]||0,1}[\*||{},3],[[]],[ref qr-1,-,-1],[{}],[sub{}^*ARGV,3]

      You're right, I completely glossed over that. Thanks for the correction; it's a very important one!

      I still stand by the assignment rather than shift though.

Re^2: RFC: Creating Dancer2 validation plugin.
by AlexP (Pilgrim) on May 05, 2022 at 09:31 UTC

    You highlighted important things there, big thanks!

    I agree, that using @_ in args is more straightforward, and it should be used instead of shift.
    Also, it's a good idea to leave comments for private methods.

    ...is there a reason you're intentionally returning undef?

    As I know, perl returns the last value from a block, so returning undef is a way to protect from random bugs. Is it unnecessary or there is a better way?

Log In?
Username:
Password:

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

How do I use this?Last hourOther CB clients
Other Users?
Others romping around the Monastery: (3)
As of 2024-04-20 02:54 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found