The title pretty much sums it up, but here's the long version anyway.
After posting a small snippet of perl code, I was told to avoid indirect object notation, "as it has several side effects". The comment referenced this particular line:
my $some_object = new Some::Module(FIELD => 'value');
As this is how I've always done it, in an effort to get with the times I therefore ask:
- What's so bad about it? (specifically)
- What are the potential (presumably negative) side effects?
- How should that line be rewritten?
I was about to ask the commenter, but to me this is worthy of its own post.
The main problem is that it's ambiguous. Does
my $some_object = new Some::Module(FIELD => 'value');
mean to call the new method in the Some::Module package, or does it mean to call the new function in the current package with the result of calling the Module function in the Some package with the given parameters?
i.e, it could be parsed as:
# method call
my $some_object = Some::Module->new(FIELD => 'value');
# or function call
my $some_object = new(Some::Module(FIELD => 'value'));
The alternative is to use the explicit method call notation Some::Module->new(...).
Normally, the parser guesses correctly, but the best practice is to avoid the ambiguity.
What's so bad about it?
The problems with Indirect Method Notation are avoidable, but it's far easier to tell people to avoid Indirect Method Notation.
The main problem it's very easy to call the wrong function by accident. Take the following code, for example:
package Widget;
sub new { ... }
sub foo { ... }
sub bar { ... }
sub method {
...;
my $o = new SubWidget;
...;
}
1;
In that code, new SubWidget is expected to mean
SubWidget->new()
Instead, it actually means
new("SubWidget")
That said, using strict will catch most of these instances of this error. Were use strict; to be added to the above snippet, the following error would be produced:
Bareword "SubWidget" not allowed while "strict subs" in use at Widget.pm line 11.
That said, there are cases where using strict would not catch the error. They primarily involve the use of parens around the arguments of the method call (e.g. new SubWidget($x)).
So that means
- Using Indirect Object Notation without parens can result in odd error messages.
- Using Indirect Object Notation with parens can result in the wrong code being called.
The former is bearable, and the latter is avoidable. But rather than telling people "avoid using parens around the arguments of method calls using Indirect Method Notation", we simply tell people "avoid using Indirect Method Notation". It's just too fragile.
There's another issue. It's not just using Indirect Object Notation that's a problem, it's supporting it in Perl. The existence of the feature causes multiple problems. Primarily,
- It causes some syntax errors to result in very odd/misleading error messages because the code appeared to be using ION when it wasn't.
- It prevents useful features from being implemented since they clash with valid ION syntax.
On the plus side, using no indirect; helps the first problem.
How should that line be rewritten?
The correct way to write the method call is the following:
my $some_object = Some::Module->new(FIELD => 'value');
That said, even this syntax is ambiguous. It will first check if a function named Some::Module exists. But that's so very unlikely that very few people protect themselves from such problems. If you wanted to protect yourself, you could use the following:
my $some_object = Some::Module::->new(FIELD => 'value');
As to how to avoid it: There's a CPAN module that forbids the notation, acting like a pragma module:
no indirect;
http://metacpan.org/pod/indirect
The commenter just wanted to see Some::Module->new(FIELD => 'value'); as the constructor.
Perl can use indirect object syntax for other bare words that look like they might be methods, but nowadays the perlobj documentation suggests not to use it.
The general problem with it is that code written this way is ambiguous and exercises Perl's parser to test the namespace to e.g. check when you write method Namespace whether Namespace::method exists.
