ajt, congrats on taking the dive into object-oriented programming. I wish I had thought to ask these questions when I first started to learn. Regarding your questions, I would assume that the programmer using your module is competent. Making too many assumptions that your programmer is a weak programmer can bloat your code. However, you still want to provide the target programmer with enough utility and information that should he or she make a mistake, it's still easy to figure out what's going on. A classic example is one of my first large OO projects. For non-recoverable errors, I had die statements. Naturally, they died when they were called incorrectly, which provided the programmer with virtually no information about the context in which the offending method died. croak would have been a much better tool as it dies from the perspective of the caller, not the callee.
Here's my take on some of your questions.
Should my module's methods croak, or return error codes when they fail?
What happens when a method returns? Is it an error that the programmer can reasonably recover from? Failure to read a config file is typically fatal. Finding out that there are no customers with the first name of 'Frank' probably is recoverable. Croak or return an error code depending upon whether or not the program should terminate.
How much internal error checking is sensible? Some external calls will fail anyway if I pass duff data.
I like to use plenty of error checking. As with the answer to your first question, what happens if there is an error? Is it recoverable? You might want to check out Class::Contract. That allows you to use Design by Contract programming for easy testing of pre- and post- conditions (and other things) and has a Class::Contract::Production version that turns off this feature in production in order to enjoy better performance.
Should I use generic methods that can both get and set, or is a single one each, e.g. $o->debug or $o->set_debug(1); print $o->get_debug?
I no longer like generic methods. Here's why:
$obj->foo( 'bar' ); $value1 = $obj->foo; $obj->baz( 'qux' ); $value2 = $obj->baz;
Did you spot the error in the above code? Syntactically it's correct, but what if $obj->baz is only an accessor and not a mutator? There is no way to tell, from the code, that you can't use this method to set a property. If most of your methods are doing double duty, but a couple are just accessors, a programmer might easily get confused as to which are which. In my code, you'll often see something like this:
$products = $obj->get_product; $product = $obj->get_product( $product_id );
In that example, the first use returns all products and the second returns only the information for a given product. That, I think, is a clean way to overload methods. However, if you tend to use the same method to both get and set a property, the above code might confuse. That's why I explicitly put a get_... at the beginning of the method name so that no one can misunderstand.
If I want to submit it to CPAN, how do I go about it? I know CPAN has instructions, but USENET seems a scary place, and I don't know if the code is worth anything to anyone else?
Read tachyon's How to make a CPAN Module Distribution.
What works best, lots of methods to set things, or via the constructor in one go?
What's the common case? Are people likely to change the foo property? If not, don't use a mutator. I'd push it into the constructor. If it's an optional property, though, you'd want a mutator. However, that's too simple of an answer; it can vary from module to module.
Code comments:
sub new { my $self = shift; my %args = @_; bless { ... }, $self; }
By convention, $self is used to refer to an object. Since you don't yet have an object in your constructor, I'd rename this to $class or $proto.
sub as_string { my $self = shift; my $mode = shift; if ($mode) { if ($mode =~ /rss/i) { ... }
Before the first if statement, I would use $mode ||= ''; or $mode = '' if ! defined $mode; (the second is used if a false value for $mode is legal and you don't want it overwritten). This will suppress warnings on your regular expressions if $mode is not defined.
$self->{_xsl_string} = _http_get($uri);
That's a tempting thing to do, but don't. If you have an OO system, you may as well take full advantage of it:
$self->{_xsl_string} = $self->_http_get($uri);
How does that help? Well, what if you have an error property in your object? If you just call the _http_get function, you can't conveniently set that error property, if needed. Further, what if you need to extend that subroutine to call helper methods. Since you no longer have $self, you can't call its methods and you've limited what you can do (some will quibble with this point as there are plenty of exceptions).
Putting all of the above comments aside, I can only say that I wish my first OO module was half as well-done as yours :)
Cheers,
Ovid
Join the Perlmonks Setiathome Group or just click on the the link and check out our stats.
In reply to Re: Thoughts On Object-Oriented Module Design. Input Sought.
by Ovid
in thread Thoughts On Object-Oriented Module Design. Input Sought.
by ajt
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |