Is "ref($class) || $class" a bad thing?

There is a discussion going on over on the perl-qa list about Devel::Cover and if 100% coverage is a realistic goal or not. The most common reason for not getting 100% coverage is the common OO-perl idiom of

my $class = ref($class) || $class;
[download]

In order to write a test that would cover this, you would have to include the following in your test code:

my $foo = Foo->new();
my $foo2 = $foo->new();
my $foo3 = Foo::new();
[download]

This would then satisfiy Devel::Cover's condition coverage fully, and cover all 3 variations:

+------+-------+-------+
| true | false | final |
+------+-------+-------+
|   0  |   0   |   0   |  Foo::new()
+------+-------+-------+
|   0  |   1   |   1   |  Foo->new()
+------+-------+-------+
|   1  |   x   |   1   |  $foo->new()
+------+-------+-------+
[download]

But it occured to me that while this would give you sought after 100% coverage, it not only did nothing to improve the quality of the test, but it allowed a (potential) bug to slip through.

Consider this code:

package Foo;
sub new {
  my ($class) = @_;
  $class = ref($class) || $class;
  return bless {}, $class;
}

sub test { print "Test" }

package main;
my $foo = Foo::new();
print $foo;
$foo->test();
[download]

It's output (IMO) is not a good thing.

main=HASH(0x180035c)
[download]

The hash that was supposed to be blessed into the Foo package, has been blessed into main instead. No error, no warning, no indication that what has happened is incorrect. Only when an attempt is made to call a method though the $foo instance do we find out what is wrong, with the not-very-helpful error message of:

Can't locate object method "test" via package "main" at test.pl line 1
+4.
[download]

Line 14 is $foo->test(), thats not very helpful. Sure I know what happened, and most reasonably experienced perl hackers will see the problem pretty quickly (especially if they are familiar with OO-perl). But that still leaves out a portion of the population which are just left scratching their heads.

Back in perl-qa, someone posted a link to this paper How to Misuse Code Coverage, which i recommend reading if you are currently using code coverage in any of your projects.

The author of the paper basically points out that code coverage tools can give a false sense of security to your tests, that bad test design can still produce good coverage. That sometimes there are errors/issues/incidents which are not actually handled in the code, and therefore cannot be tested, and hence your code coverage tool cannot tell you that the code that ought to exist is covered.

This then brings me back to my $class = ref($class) || $class; and achieving 100% coverage with it. In reality, many modules out there call other methods in their constructor, and hence will die when you call Foo::new(). This can be tested easily like this:

eval { Foo::new() };
ok(!$@, "Foo::new dies when called as a function");
[download]

You can get your 100% and sleep comfortably at night knowing that all is right in the world.

But what about all the modules out there which don't call other methods in their constructors? Maybe, it time to re-think this common idiom? What say you O fellow monks?

-stvn

Back to Meditations