Re: OO: Leaving a constructor midway?

[jest]

Thanks for the various thoughts. I appreciate, if not fully understand, the technicalities about exactly when this is an "object" and whether or not it's really a "constructor" if I leave midway.

I don't have any other OO background, so my approach to this was a DWIW one. The functionality of every program that will use this module involves having a user logged in with some info in a session; if the user isn't logged in there's no point going further. When, in my original calling program, I say

my $book = MyPackage->new( # stuff );

, my purpose is not "to create an object", it's "get started, with whatever that entails", and if that entails logging the user in, so be it. Throwing an exception seems to force me to add an extra step to every single program that uses this module; instead of the above line, I now have to say something like

my $book = MyPackage->new( # stuff );
print CGI::redirect(-uri=>"login.cgi?msg=$book->{error_msg}")
   unless $book->{login_successful};

or whatever, i.e. adding an extra line of identical code to every program.

Indeed, in practice it might even be longer, since in some circumstances I need to check whether the user has the right access level. Then I'll have yet another similar or identical line to every program:

print CGI::redirect(-uri=>"login.cgi?msg=$book->{no_access}")
  if $book->{user_access_level} < $book->{required_access_level};

I had wanted to put the code in the module, instead of the programs, specifically to avoid this. But if this is bad style, then it's bad style; the reason I asked is because I thought it might be, and I'll do it by throwing exceptions instead.

[dragonchild]

Why not do this:

my $book = MyPackage->startup( $stuff );

package MyPackage;

sub startup {
    # do stuff here, like creating the object

    # do your check for authority here and exit, if appropriate

    # Do more stuff here

    return $book;
}

This is the method used by classes like DBI. Calling it startup() makes it more clear to your maintainer (which may be you!) what exactly you're doing. (Which is why I suspect Tim Bunce chose connect() instead of new() for DBI.)

[jest]

Silly question, but: what's the difference? I mean, I see that there's a difference behind the scenes, in that this suggestion involves calling a separate constructor that really does bless the object, and then does the login business etc. But from the point of view of the user, it's the same: you call some method (whether called "new" or "startup"), and either bounce to a different program, or get back an object blessed into MyPackage. After all, you are the one who so recently said to Abigail-II that if I call a method and get back something that quacks like an object, it's a constructor, so....

If this version makes the OO gods smile upon me, I'm happy to go with it, but I'm just curious. Several people seemed to react with horror to the idea of a constructor exiting the program, so IDG why it's not a horror to having a constructor-like thing exiting the program.

[dragonchild]

The difference here is that startup() wraps the constructor. That constructor is still (possibly) available to the client. This is an important distinction. This is (kinda) how DBI does it. Now, DBI's constructor(s) are not visible from the outer-most client, but it is used by the DBD:: class that DBI is providing a connection through.

Now, DBI is a pathological example, cause it does so much more than just provide an object. But, I think you get my point.

Also - who cares if the "OO gods" smile on you. It's your code, you have to maintain it, and you're the one who has to look at yourself in the mirror. If you're happy with it, the the heck with everyone else.

[pdcawley]

What you describe in that code is not catching an exception, it's handling a return value, which means that it's doing the work too near to the failure. Have your login_user method use a real exception class, so if login fails you'd do:

...
die Exception::NoUser->new(-requested_uri => $request->uri);
...

Then, way up at the top level, in your basic handler method that everything uses, you would have:

my $result = eval { $app->process($request) };
if ($@) {
  $result = ref($@) ? $@ : 
    Exception::Unknown->new(-requested_uri => $request->uri,
                            -message => $@);
}
$self->emit($result);

There are big problems with your code though, in that it makes assumptions about how it's being used, which makes it very hard to write tests for it. For instance, what is it that's inherent in your Book object that means it needs to know it's being used via a webpage? Slightly more controversially, why does it need to know it's being fetched from a database? As someone else suggested, maybe your application's process method needs to do something like:

sub process {
  my($self, $request) = @_;
  local Exception::current_uri = $request->uri;
  $self->get_resources($request);
  $self->authenticate_user($request);
  $self->authorize_user($request);
  $self->generate_response($request);
}
sub get_resources {
  my($self,$request) = @_;
  # Fetch your book from the database.
  $self->set_book($book);
}
sub authenticate_user {
  my($self, $request) = @_;
  my $user = $self->make_user_from_request($request) or
    die Exception::NoUser->new;
  $self->set_user($user);
}
sub authorize_user {
  my $self = shift;
  $self->user_has_required_access($user) or
    die Exception::InsufficientAccess->new(user => $user);
}

If you set up your inheritance tree (or compose your objects) correctly, you'll only have to override one or two methods for each distinct page. And you'll have a collection of methods that you can test without having to pretend to be a webserver all the time, and that's a big win.