As Aristotle has pointed out, the HTML shouldn't be in your code. There are several ways to handle that. One is to not return anything that deals with formatting but instead have your program pass the results off to a template and the template decides the format. This has the advantage of keeping the formatting in the spot that it logically belongs (and if you use CSS, you can reap even greater benefits).

Another idea which I don't like as much, but which might be applicable for you, is to turn this class into a base class and have presentation classes that inherit from this and handle the formatting. I don't like that because it seems like bad design and isn't (in my mind) what inheritance is for. The benefit would be another method of controlling the presentation and it would allow you to override many of the constant values that you have hardcoded into the system. For example, if someone fails to login 3 times in a row, maybe you want the lockout time to be an hour. It all depends upon the business rules you're dealing with. Whenever you see hardcoded data like yours in a program, consider whether or not you'll ever need to override those values.

If you do wish to subclass this, you'll need to switch to the two-argument form of bless.

#constructor
sub new
{
    my $class = shift;
    my %self = map {$_ => undef} qw/user_id login_valid error_type err
+or_string/;
    bless \%self, $class;
}
[download]

Cheers,
Ovid

New address of my CGI Course.
Silence is Evil