Refactoring prototypes - what am I going to break

[throop]

Brethren,

I'm refactoring legacy Perl code that uses prototypes. The standard wisdom on prototypes is Don't.

All the prototypes in this code are for one or two scalar args; e.g.

sub files_identical($$) {
   my ($file1, $file2) = @_;...

sub inspection_current($) {
   my $file    = shift; ...

In other words, the only char inside the prototype parens is a '$'.

In that case, is there anything that might break if I removed the prototypes?

throop

[moritz]

My (limited) understanding of prototypes is that they modify the way perl parses your code.

Which means that

sub foo($) { ... }

bar foo $arg1, $arg2

parses as bar(foo($arg1), $arg2) Whereas without protoypes it parses as bar(foo($arg1, $arg2))

So you could break things by just removing prototypes.

Update: A code example to test this:

sub foo($) {
    print "foo: $_\n" for @_;
    return "returned from foo";
}

sub bar {
    print "bar: $_\n" for @_;
}

bar foo 1, 2;
__END__
# with prototype on foo:
foo: 1
bar: returned from foo
bar: 2
# without prototype:
foo: 1
foo: 2
bar: returned from foo

Another way to test this is using B::Deparse: perl -MO=Deparse -e 'sub f($) { 1 }; sub bar { 2 }; bar f 1, 2'

[tye]

You'd need to replace unlikely code like foo(@bar) with foo(0+@bar). More likely problem would be foo(bar()) which might need to be made foo(scalar bar()). The $ in the prototype forces scalar context upon the arguments while w/o prototypes the arguments are evaluated in list context.

[Sidhekin]

moritz already demonstrated how precedence might break.

Further, context might break. (Update: And tye said so already, while I was writing this. Eh.)

Given those prototypes, files_identical( foo(), bar() ) currently means files_identical( scalar(foo()), scalar(bar()) ). Dropping prototypes means putting foo() and bar() in list context instead.

If foo() is (or returns something that is) context sensitive, this may break.

(Likewise, inspection_current( @array ) will, given the prototype, get the number of elements in @array. Without the prototype, it will get the first element. Given that this argument is called $file, this is a rather far-fetched scenario, but should serve to illustrate context sensitivity.)

In this case, the standard wisdom on prototypes may well be trumped by the standard wisdom of legacy code:

If it ain't broke, don't fix it.

[halley]

The standard wisdom on prototypes is: Don't.

The standard wisdom on prototypes is: Don't use them unless you really know what they're doing.

For old Perl code written by peers, you can judge if they had a clue. Usually they didn't; usually they think it's like C prototypes. Review how the functions are getting called. Test each change. For well-established code written by Perl gods or regular CPAN-submitters, you can usually leave them alone, they're there for a reason.

[throop]

> or old Perl code written by peers, you can judge if they had a clue.

Yeah, well I judged that, and that's why I wanted to get rid of the prototypes.

> If it's not broke, don't fix it.

Well, it works, but it's not very maintainable. Should it be refactored?

[halley]

Well, it works, but it's not very maintainable. Should it be refactored?

You refactor when the code requires change. When it requires change, you use that opportunity to make it better. You don't take the tire off unless it's flat, or you have to rotate them all.

[cdarke]

Why remove them? Are they causing problems? If it ain't broke don't fix it

Perl prototypes are not like C/C++ prototypes (which many programmers do not understand anyhow) they force the context. Their strength is in detecting errors at compile-time, which is particularly useful when passing references to specific types, like arrays and hashes. Sure, you can check the correct type has been passed using ref, but at best you can only generate a run-time exception in this way.
Their weaknesses are that they are ignored by OO programs, and by most people, since they are optional. This means that their subtlties are not generally understood, life's too short.