All the Perl that's Practical to Extract and Report

Discuss the Pros and Cons

Ovid on 2009-01-13T17:36:49

This:

sub concrete_classes {
    my $self = shift;
    return grep { ! ( /::Base::/ || /::SUPER$/ ) } $self->domain_classes;
}

Or this:

sub concrete_classes {
    my $self = shift;
    return grep { not $_->is_abstract } $self->domain_classes;
}

Which of those would you choose and why? Are there arguments for the other? What are the downsides of the "wrong" choice? Would you do something entirely different (tough to know with so little info)? I've already got my opinion, but just want to hear your point of view in case I've missed something obvious.

Update: Fixed a couple of typos which were pointed out.

Abstract Classes

Stevan on 2009-01-13T18:20:51

I suspect you mean $_->is_abstract and not $self->is_abstract. Beyond that I think I need to know more of the context in which this is used.

- Stevan

Re:Abstract Classes

perigrin on 2009-01-13T18:38:42

Also I think he needs the opening '(' for { ! ( /::BASE::/ || /::SUPER$/ ) }.

Re:Abstract Classes

Ovid on 2009-01-14T07:42:59

Each method attempts to do the the same thing: return a list of non-abstract classes. The first method inspects class names and uses a heuristic to find out if they're abstract classes. The second method asks the classes if they're abstract classes. How this is used could vary, obviously. As an abstraction, there are any number of reasons why you might want a list of non-abstract classes.

hmm

jsut on 2009-01-13T18:26:55

The second option is probably more maintainable, especially if is_abstract is implemented in some common base class somewhere. If you've got grep's like that peppered throughout your code for some reason, even more so. If you don't, then it might not make sense to add that level of abstraction.

Are they even equivalent?

mauzo on 2009-01-13T18:44:48

That would require that you never have a concrete class inheriting from another concrete class, which feels like a very bad thing to bake in.

Of course, I don't know how those ::SUPERs get in there. It's possible they can only arise when the class is already known to be abstract; but that just makes the whole thing even more obscure.

So, the second :). I would probably want the is_abstract method to be a sprinkling of sub is_abstract { 1 } and { 0 }s rather than simply that expression, as well.

Depends

jplindstrom on 2009-01-13T22:43:16

Well that probably depends on a whole lot of things that aren't evident from the examples or your description.

So I have no clue.

Re:Depends

jrockway on 2009-01-19T20:55:02

Why not:

  return grep { $_->can('new') } $self->domain_classes;

Also, it's somewhat silly to use the word "class" for something that's not a class. (An abstract class is not a class.) When you think "abstract class" and you are a Perl programmer, you really want roles.