in reply to Refactor method calls or not?

No need to obscure things with AUTOLOAD. 
%ALLOWED_TABLES = map {$_ => 1} qw(company financialDiary);

sub add_data {
    my ( $self, $table, $data ) = @_;
    die "bad table!" unless $ALLOWED_TABLES{$table};
    my $data = $self->_generic_insert( $data, $table );
    $self->{ _dbh }->commit if ! $self->{ _error };
    return $data;
}

[download]
Of course you have to change your method calls, and maybe this is a problem for you.

Regarding transactions, I think the transaction control belongs in the code that is calling these methods, not in the methods themselves. These data access methods don't know anything about the context they are being called in.

UPDATE: fixed array name mismatch and changed to hash

Replies are listed 'Best First'.
(Ovid) Re: Re: Refactor method calls or not?
by Ovid (Cardinal) on Jan 19, 2002 at 00:00 UTC

    perrin wrote:

    Regarding transactions, I think the transaction control belongs in the code that is calling these methods, not in the methods themselves.

    There are some great ideas being tossed around here, but I want to address this issue specifically. In this system, I have what I think of as four layers:

    1. HTML Templates (via Template Toolkit)
    2. Main Programs
    3. Database API
      • Generic database parent class for all subclasses
      • Security subclass
      • Modify subclass (this is anything that changes the database and it has its own db user)
      • Select subclass (anything that only retrieves data)
    4. The actual database

    The idea here was to have the main programs grab or update data and pass the results to the HTML template, but have none of the programs allowed to touch the database directly. Item 3, the database API, is the database "gatekeeper", if you will. We have so many old projects where database code was strewn throughout the system that I felt grouping everything like this would be much safer. Am I wrong in my thinking? By having the calling code (item 2 above) handle the commit, I then move database specific code out of the designated layer. This doesn't seem appropriate, but I'd be happy to hear counter-arguments.

    Cheers,
    Ovid

    Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

[reply]
      That's a similar architecture to what we did at eCircles. We decided to create logical database "requests" that mapped to one or more stored procedure calls with explicit input and output variables.

      This means that the application code doesn't know anything about the database - it only knows about these database requests, which allowed us to decouple the applications from the database somewhat.

      As for transactions, with Sybase at least we managed to never have a transaction that spanned multiple database requests. I realize that this may not always be possible, but keeping transactions short helps tremendously on general throughput for the database.

      Michael

[reply]
      I've had the same thoughts about this. However, to make database commits you have to know the big picture, and your data access objects don't.

      One approach would be to make a set of "action" objects that are allowed to manage transactions and call the data objects, but have no web-specific code in them. This would be similar to what the Java folks call a "session facade". In Java this is done because EJB performance sucks rocks, but in this case it would simply be an organizational tool.

[reply]
Re: Re: Refactor method calls or not?
by Juerd (Abbot) on Jan 19, 2002 at 00:04 UTC
    There is no $spoon, er.. $type :) Did you mean $table there?
    Your grep either returns all values (if $type is true), or none at all (if $type is false). And die is evil outside the main program.
    I'd also use a lookup hash to avoid having to iterate over the array over and over.
    use Carp;
...
my %tables;
@tables{ qw/foo bar/ } = ();
sub ... {
    ...
    croak 'Disallowed table' unless exists $tables{$table);
    ...
}

    [download]

    2;0 juerd@ouranos:~$ perl -e'undef christmas'
Segmentation fault
2;139 juerd@ouranos:~$

    [download]

[reply]
[d/l]
[select]
      Thanks for catching those. I should have proofread before posting. And of course that should have been a hash. Don't know what I was thinking.

      The die is just a stand-in for whatever Ovid uses for exception handling.

[reply]
Re: Re: Refactor method calls or not?
by Anonymous Monk on Jan 19, 2002 at 00:06 UTC
    You don't need to change method calls, just make the old methods wrappers for the new super-method. This way the code stays readable (and you don't need to make many changes) but the methods do not duplicate code. Mine is the voice if INexperience though. :)
[reply]
      Unless Ovid thinks he'll have to go back and differentiate these methods again in the future, changing the calls to the method is probably not much more difficult than a search and replace in the calling code blocks. If he has already bounds-tested and otherwise proofed the data elements, a more generic insert/update method is called for-- and it will be faster to change the calling code than to refactor a long list of unique methods to call the generic method. If he has *not* proofed his values, though, he might want to leave these routines with unique names so that he can validate at this point-- which is probably too late {grin}.

[reply]

        For the record, I have done bounds checking on everything. I was, however, a bit ambivalent about exactly where to put it. Data validation is primarily done at the untainting level, as much of the data is free-form. There are some specific instances where validation is a bit tighter ("does such and such a key actually exist", for instance, or "do we have a valid date"), but I opted to have that in the calling code rather than in the database API layer. I didn't want to duplicate validation in both the calling code and the API, but I still wound up with some extra validation in the API for really persnickety problems. I think I need to go back and do more reading on system design, though. I'm not quite happy with how everything got laid out.

        Side note: as for perrin's node about exception handling, they're handled either in the calling code or in the database parent class (using CGI::Carp. Since this is the largest system that I have build to date, it's been quite a learning experience.

        Cheers,
        Ovid

        Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.

[reply]

In Section Meditations