byterock

Well getting close to the end of this little series and today I though it would be good to do a little code review to see if I could fined any holes.

So there was some suggestion about a new namespace 'CheckCallerRoles' and another 'AllowedMethodRoles' but I think as I cut down on the number of files I need I will just keep the present setup besides it is easy to understand.

Now there is the question of the actually API names themselves

1. requires and
2. one_of

Now looking at the semantics of these two I think a change may be in order. Now I do have the option 'requires' is a another word for 'needs' but that doesn't read too well and 'must_have_roles' is a no good either. I could also try 'required' which is a little less in the passive voice so I think I will finalize on that.

Now 'one_of' is not 100% in my books but is close as alternatives quickly become wordy and the secret of a good API is should be clear and concise. So stuff like 'with_one_of_these', 'has_one_of' are saying the same but with just more words so I will now go with

1. required and
2. one_of

Now the other little hole I sort of still do not like is this

 
 my $author_sub = '_authorize_'.$key;
      next
        unless ($self->can($author_sub));
      $self->$author_sub($requires->{$key},$instance);

That 'can' in there to filter out my API. Well in a perfect world I would make it fully configurable though most likely a YML file but I think that would just be overkill and heck the world if full of compromises and sometimes the code that compromises is much better code. So I will keep that for now.

Now we have my validation part.

 

   unless (((exists($requires->{requires}) and ref($requires->{requires}) eq 'ARRAY')) or

              ((exists($requires->{one_of}) and ref($requires->{one_of}) eq 'ARRAY')));

Now perhaps this can be fixed up a little because as it stands I have to add in a new 'or' here each time I expand my API. Well I do not see the API growing too much even as we been using it for a few weeks now we have yet to find a use for any other 'keys'.

I suppose however, some day a smarty pants out there will want something like 'has_at_least_two' or perhaps 'has_no_more_than_three' but I will let them add that in as they need them.

Well that is about it me thinks time to move this Moose