Re: Idea on a Base class API

[stephen]

First, I just want to say that I really enjoy this kind of post, and ++ dragonchild for doing this.

That said, I'm not sure I understand what the advantage is for a single get(), set() etc. method. If you're objecting to the additional work of defining all of the individual accessor/mutator methods, then there are implementation-based ways to eliminate this, like Class::Accessor, Class::Struct, and Class::MethodMaker.

I agree that it would be nice to have a facility for checking data types in members. Class::MethodMaker offers this for class types.

I'm a bit worried about the define_attributes() method. What is the expected behavior when it's called on a class where there are already instances out there? Does it suddenly alter the class behind the scenes? I think that could get pretty confusing.

I'd like to see a contract like this. However, my personal preference would be toward labeled get_foo() and set_foo() methods, since they're clearer to my eye. Plus, having a separate method for each member means that I can enforce any kind of consistency check I'd like without tacking on conditionals. If "foo" should be a multiple of pi, for example, and I want to have a method that tests this on sets, I don't want to have to override set() and insert an if ($name eq 'foo') { ... } statement, since avoiding confusing conditionals is one of the things that OO is supposed to help you avoid.

Anyway, just my thoughts on the matter.

stephen

[dragonchild]

I agree that there are a number of ways by which the definition of individual accessors can be defined. However, I'd like to give a snippet to demonstrate the english-ness of this method.

my @attribute_names = ('attr1', 'attr2', 'attr3');
foreach my $attribute (@attribute_names) {
    my $value = $self->get($attribute);
    print "$attribute => $value\n";
}

This is as opposed to how it would have to be using get_foo() or set_foo(). That would look something like:

my @attribute_names = ('attr1', 'attr2', 'attr3');
foreach my $attribute (@attribute_names) {
    my $function_name = "get_$attribute";
    my $value = $self->$function_name;
    print "$attribute => $value\n";
}

I ask you ... which is clearer? Of course, there's an even clearer way to do that exact same thing:

my @attr_names = ('attr1', 'attr2', 'attr3');
my @values = $self->get(@attr_names);
print "$attr_names[$_] => $values[$_]\n" for (0 .. $#attr_names);

Personally, I think that Example 3 is the cleanest of all.

Now, the biggest problem I have with Example 2 (and its cousin, $self->$attribute;) is that you're using a string as a function name. Can you guarantee that this function exists? Maybe, but it's a lot of work, and you restrict your options. For code that will always handle exceptions gracefully, you want to have your function names hard-coded. That way, you will always be executing something.

A nifty benefit, too, is that you're expanding your options of viable code, as you can see from Example 3.

The expected behavior of define_attributes() when called on a class with instances out there already is ... well ... not determined yet. Certain implementations of objects mandate that the attributes will get added immediately, and others mandate that it has to be done by hand. I, personally, would say that if you call define_attributes again, that you will add new attributes to all instances, present and future, and that's just the way it has to be.

A more important problem is what happens when you call define_attributes() on the same attribute. Or, what happens when Foo overrides the allowable types on attrib1 inherited from Bar. These are the decisions that have to be put into the contract, and that's why I'm putting the draft of the contract out there for everyone to read and comment on. :)

Now, to your objection about needing to add extra conditionals when dealing with set() ... that's a good objection, and one that I hadn't really thought of. Three possible answers immediately come to mind:

1. Do something like $self->set(foo => $val) unless $foo % PI; ... put the conditional outside the call to set().
2. Add something to the allowed type, something like MULTIPLE(x) where x is something you've supplied. So, you could have:

   FooClass->define_attributes(
       foo => { Type => 'MULTIPLE(x)', Default => 0 },
   );
   

   And, no, I'm not quite sure how that would be implemented. But, that's not the point in this discussion. The idea is to figure out what is desired.
3. Create a set_foo() method that will encapsulate Option 1.
Do I know which is the best answer? Not yet. If I were to implement something right now, I'd use Option 3. (In fact, I have used Option 3, in a slightly different context.)