This may not be so good of an idea if using multiple instances of the class in a single program. Even with persistent connections, it seems too cumbersome to pass in database info to the constructor each time. If you're not using persistent connections and you create 5 objects in a script, each object will create a new connection to the database.

This also doesn't scale very well if using another similar class in the program. Again, multiple handles to the same database for each guest object and each XYZ object and you have some pretty greedy perl code.

If you're only using one class to access the database, then maybe use a class variable:

package Guest;
use vars qw($DBH);

sub new {
  $DBH ||= DBI->connect(...);
}

END {
  $DBH->disconnect();
}
[download]

The original code showed passing in a reference to the database handle to each method and that would work fine. A better way may be to pass that in to the constructor and have each object store that instead. But if you do that, make sure you don't have the destructor close the database handle either. Doing database opens/closes in a central location (in this case, the main program) seems like the cleanest way.