Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Is the subroutine test1 ugly?/uglier than the others?

#!/usr/bin/env perl
use warnings;
use strict;
use 5.10.1;
use Data::Dumper;

my $ref = { one => 1, two => 2, three => 3 };


sub test1 {
    my ( $ref ) = @_;
    $ref->{three} = 4;
}
test1( $ref );

sub test2 {
    $ref->{three} = 4;
}
test2( $ref );

sub test3 {
    my ( $ref ) = @_;
    $ref->{three} = 4;
    return $ref;
}
$ref = test3( $ref );


say Dumper $ref;
share|improve this question

migrated from stackoverflow.com Feb 23 '13 at 7:58

This question came from our site for professional and enthusiast programmers.

3  
The argument in test2( $ref ); is ignored. –  aschepler Feb 19 '13 at 15:18
1  
@HunterMcMillen I had not thought of it. –  sid_com Feb 19 '13 at 17:04

1 Answer 1

up vote 7 down vote accepted

Here are some considerations when writing such functions:

  • By passing the function all of the variables it needs (test1), you don't need to rely on global variables. This makes the function easier to test, more predictable (no side effects) and easier to refactor as you can move it to another module or file easily. If it relied on global variables, it would be broken if the variable is moved or its data structures changed.

  • Returning the original reference is not necessary for test3, because the original $ref is changed. Returning is useful if you want to chain function calls.

  • Modifying the arguments passed your function directly can be surprising to the function's caller. If you do this, make it clear in the documentation. If you actually want to make a new copy of the object, modify it and return it, use something like Clone. Of course, this can be slow.

  • Just in case you are writing object accessors/mutators: please consider using a module such as Class::Accessor or the more heavyweight but extensive Moose which can generate such functions for you automatically.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.