Method parameters validation and inheritance

roman has asked for the wisdom of the Perl Monks concerning the following question:

Dear monks,

this is not a serious problem, but I come across its variants quite frequently.

I have a method (say open_for_reading), which accepts hashref of parameters. Since it is quite important method I need all of its parameters to be understood. The open_for_reading method is overriden in a couple of subclasses, each of them understands more parameters.

my $context = $file->open_for_reading( {} );

my $context2 = $file->open_for_reading( { 'path' => $file } );
# OK
my $context_derived
    = $file_derived->open_for_reading( { 'path' => $file, 'join_longca
+lls' => 1 } );

# this should fail
my $context_derived = $file->open_for_reading( { 'join_longcalls' => 1
+ } );
[download]

Without the validation the code is something like:

package MyBaseClass;

sub open_for_reading {
    my ($this, $params_ref) = @_;

    $params_ref ||= {};
    return $this->get_context('path' => $params_ref->{'path'} || $this
+->path);
}

package MyDerivedClass;

sub open_for_reading {
    my ($this, $params_ref) = @_;

    $params_ref ||= {};
    my $context = $this->SUPER::open_for_reading();
    if ($params_ref->{'join_longcalls'}){
        ... modify context
    }
    return $context;
}
[download]

For parameter validation I found Params::Validate from CPAN. The only way I was able to add the validation is like this:

package MyBaseClass;

sub open_for_reading_validation_spec {
    return { 
        'path' => 0,
    };
}

sub open_for_reading {
    my ($this, $params_ref) = @_;

    $params_ref ||= {};
    Params::Validate::validate($params_ref, $this->open_for_reading_va
+lidation_spec);
    return $this->do_open_for_reading($params_ref);
}

sub do_open_for_reading {
    my ($this, $params_ref) = @_;

    return $this->get_context('path' => $params_ref->{'path'} || $this
+->path);
}


package MyDerivedClass;
use base qw(MyBaseClass);

sub do_open_for_reading {
    my ($this, $params_ref) = @_;

    my $context = $this->SUPER::do_open_for_reading($params_ref);
    if ($params_ref->{'join_longcalls'}){
        ... modify context
    }
    return $context;
}

sub open_for_reading_validation_spec {
    my ($this) = @_;

    return {
        %{$this->SUPER::open_for_reading_validation_spec},
        'join_longcalls' => 0,
    };
}
[download]

Instead of one method I have three:

interface method which is not overridden in subclasses
executive method which is overriden in subclasses
method returning the validation specification

I can live with three methods, but besides open_for_reading there are also open_for_writing, open_for_processing, ... so whole interface suddenly "triplicates".

Do you think is there a different solution?

Even without the validation there is a "naming" problem of its own. I have an interface method which does initialization and cleanup, and between these two blocks is a code to be overriden in subclasses.

sub method {
    my $this = shift;
    
    $this->do_some_initialization;
    $this->do_something_which_can_be_overriden_in_subclasses();
    $this->do_some_cleanup;
}
[download]

I put the inner block into separate method, but I often have problem to name it. Do you use some naming convention for the "executive" methods? Prefixes (do_, exec_)? Postfixes (_body)?

Comment on Method parameters validation and inheritance Select or Download Code

Replies are listed 'Best First'.
Re: Method parameters validation and inheritance by kyle (Abbot) on Sep 18, 2008 at 15:22 UTC
I would probably try to structure my code so as to avoid the use of `$self->SUPER::whatever()`. Then I wouldn't need to have a separate method (that subclasses override) to store the spec for Params::Validate. You can still pull the spec out of the method itself using lexicals and closures. Read more... (889 Bytes) If you have a bunch of stuff in `open_for_reading`, decompose it into private methods that the main `open_for_reading` then calls. The overridden `open_for_reading` in the child class can then call the same methods (or some subset of them). They enforce their own parameters, and callers will have to abide by them but won't have to have their own copy of them. Since we're not talking about real code here, I'm necessarily doing a lot of handwaving. Please let me know if I'm not getting my point across. As for part two of your question, I'd probably do this: `sub method { my $self = shift; $self->method_init; $self->_method; $self->method_cleanup; }` [download] If you don't like the "`_method`" naming, you might pick another suffix ("`_exec`", maybe). I'd prefer a standard set of suffixes over a standard set of prefixes. Another approach would be to make that group of methods into an object. It can do its initialization during construction, cleanup in `DESTROY`, and the method itself can keep its name. Then you subclass it and override that one method. I don't think that's always a good idea, but it definitely takes the clutter off somewhere else, and that's often a good thing.	[reply] [d/l] [select]
Re: Method parameters validation and inheritance by Herkum (Parson) on Sep 18, 2008 at 21:55 UTC
In referen to naming conventions, I have a simple rule, make it simple to read, it will be simple to program. This basically means, Avoid abbreviating. This is not the 'Wheel of Fortune' you can spell out whole words. Avoid writing sentences. You should be able to name a method to encapsulate its utility in no more than 3 words. Anything more it start to sound like a government agency. Name your method within the context it is being used. You know what subject you are talking about, don't be verbose about it. Taking the example of some of your code above, `my $context = $file->open_for_reading( {} ); ## Change to ############################# my $context = $file->reading;` [download] `my $context_derived = $file_derived->open_for_reading( { 'path' => $file, 'join_longca +lls' => 1 } );` [download] What the hell is 'join_longcalls'? For some reason it sounds like a pirate, and it is basically incomprehensible so it might was well be a pirate. Make it read like you actually write, don't make something up and hope people understand what the hell you are talking about.	[reply] [d/l] [select]
Re^2: Method parameters validation and inheritance by TGI (Parson) on Sep 19, 2008 at 07:09 UTC
This must that "pirate software" I keep hearing about! Really, there's no reason to stop there! `use Acme::Lingua::Pirate::Perl; ... my $context_derived = $file_derived->open_for_reading( { 'path' => $file, 'join_longcalls' => 1 } ); scuttle her matey "Avast! Context error - " $file_derived->error if $context_derived be '' curse ye deadlights;` [download] Of course, roman may prefer other dialects. Time for a brief lapse into serious commentary, and nitpicking. The problem I see with "join_longcalls" is that isn't "join_long_calls". Consistent formatting of parameter names, variables and function/method names is important. Otherwise you wind up with PHP. TGI says moo	[reply] [d/l]
Re^3: Method parameters validation and inheritance by roman (Monk) on Sep 21, 2008 at 18:41 UTC
Thanks for your comments. I should probably give some longcalls explanation. Code sample given is a simplified but real world example: `$file` encapsulates a binary file (log) produced by a phone switch. It contains cdrs (Call Data Records) describing voice calls carried out. The file is used as a source for several target applications (billing, dwh). If a call continues longer than say half an hour it is logged by parts. If `join_longcalls` is set for `open_for_reading` only complete calls are produced by `read_cdr` and the total duration of the call is summed up. So the `join_longcalls` flag really causes the long calls to be joined. In such case I am not sure whether to use `join_long_calls` or `joins_long_calls`? I think `open_for_reading` is clearer than plain `reading` since I try to use imperative for my subs. As kyle suggested, I could avoid some of the "SUPERs" and turns the code into: `my $context = $file->open_for_reading({'path' => $path}); $context->joins_long_calls(1); # or better like this, # because there is no property join_long_calls $context->join_long_calls;` [download] The `open_for_reading`, actually returns Class::Prototyped based object, so `join_longcalls` - whatever its implementation is - actually wraps `read_cdr` method. This approach has some advantages: The interface of open_for_reading remains same across classes. It is more self checking - since the parameters were turned into method calls, which fail when called but not defined. This approach has also some disadvatages: Although `$lc->join_long_calls` can be called anytime, it make sense only immediately after open. When I have target file (representation of the billing, dwh file) I could have previously used a declaratory `source_params` method containing the parameters to be passed to `open_for_reading`, now I had to redefine some `open_source` method.	[reply] [d/l]
Re: Method parameters validation and inheritance by GrandFather (Saint) on Sep 18, 2008 at 11:49 UTC
Using attributes may provide another avenue to explore. I had a brief play with them for this sort of thing in Attributes with new to manage parameters Perl reduces RSI - it saves typing	[reply]
Re: Method parameters validation and inheritance by talexb (Chancellor) on Sep 22, 2008 at 16:27 UTC
Interesting problem. If I understand the situation correctly, you have a base new method that has a few parameters, and a sub-class new method that has a few more parameters. I would probably approach this in the sub-class new method by extracting the sub-class parameters from the hash, reserving them, calling the super-class' new method with just the parameters it understands, and then continue processing with the previously reserved parameters, operating on the new object. The destroy method may also have to do some destroy local and destroy super work. In keeping with OO precepts, you want to make this process invisible to the base class. Alex / talexb / Toronto "Groklaw is the open-source mentality applied to legal research" ~ Linus Torvalds	[reply]