Thursday, 10 June 2010

When not to shift...

I'm not a big fan of 'shift @array'. And I have just sealed why I think that is.

Anyone who has looked at my code will see that my methods always use array assignments
sub my_method {
my ( $self, $arg_refs ) = @_;
}

Even if it is just $self, rather than
sub my_method {
my $self = shift;
}

Today, I further found another bug whilst testing. I'm trying out Test::MockObject, and know that I want to return an arrayref, with 1 element in the array.

The code loops through 8 times, and I expect a result from this array each time, since the webservices I am mocking would provide this.

However, in my tests, I get a result of 1 true, rather than 8.

Why is this?

I check the arrayrefs are the same with some debug (they are). I check the array length each time through. 1st time, 1. All other 7, 0.

The reason is that instead of just assigning
my $ref_seq = $arrayref->[0];

I have shifted
my $ref_seq = shift @{ $arrayref };

So of course, I have within the code removed the element off the array, rather than leave it there, and since $arrayref goes out of scope in the code as soon as $ref_seq is assigned, either works.

This is one time when I would expect the 'real world' to have still been fine, but my 'test environment' can't cope.

Obviously, there are times when shift is most appropriate, i.e. you deliberately need to truncate out the first element because you must not allow that to get somewhere else as the array carries on, but that niggling little voice that keeps telling me never to shift has finally had a reason to give me.

Still, liking Test::MockObject :)

2 comments:

Unknown said...

sometimes you have to shift. Like if you use Moose method modifier override with super() because you can't modify the array passed to super. so you have to shift $self but you can't shift anything else.

Unknown said...

Thanks for your response. Of course, there are always times you need to shift. However, in my example (the code it came from), it was one time that shifting was totally unnecessary, and actually caused tests to break.

You are absolutely right about the Moose method modifier. One time when it is truly the right thing to do.