Wednesday 16 March 2011

Interesting bug

I have just discovered an interesting bug, which took a lot of staring at to fathom out exactly what is wrong here:

if ( $self->is_paired_read() ) {
my $args = {
dir => $arg_refs->{dir},
position => $position,
read => $self->general_values_conf()->{sanger_read_two},
job_dependencies => $arg_refs->{job_dependencies},
control => $is_control,
is_spiked_phix => $is_spiked_phix,
};

if ( $self->is_indexed() ) {
$args->{read} = $self->general_values_conf()->{illumina_read_two};
}

$bsub_command = $self->_calibration_table_bsub_command( {
dir => $arg_refs->{dir},
position => $position,
read => $self->general_values_conf()->{sanger_read_two},
job_dependencies => $arg_refs->{job_dependencies},
control => $is_control,
is_spiked_phix => $is_spiked_phix,
} );

if ( $self->verbose() ) {
$self->log( $bsub_command );
}

push @{ $job_ids }, $self->submit_bsub_command($bsub_command);
}


Some of you may have spotted it straight away, but others may not.

The problem comes from a bit of

1) copy and paste programming (there is a read 1 as well)
2) not paying attention

Had I not had the if ( $self->is_indexed ) block, then Perl::Critic would actually have saved me here, with a unused variable error, but because $args (lexically scoped) , is used in the block, we skip that.

Simply, when you go to the trouble of defining your args outside of the call to the method, then use the args, don't rewrite. (Bangs head against wall to try to remember the fact).

The bug in production is that we always get a calibration table name with read 'sanger_read_two', when it should be read 'illumina_read_two' on an indexed run.
(These are config variables, which actually equate to 2 and 3 respectively)

So a change to:


$bsub_command = $self->_calibration_table_bsub_command( { $args } );


and all is fixed.

5 comments:

Unknown said...

You probably want to write "...->_calibration_table_bsub_command($args)" or "...->_calibration_table_bsub_command({ %$args })", no ?

Max.

Unknown said...

You're right. A typo in a bugfix - Doh!

Unknown said...

Java Application Development OTS Solutions : ISO 9001 & Microsoft Certified Software Outsourcing & Development service provider company offering software Application development , IT outsourcing services to its clients across the globe.

Anubhuti Mittal said...

Techgropse as a Java Development Company agrees to the fact that Java is a dominant programming language used in the distributed environment of the internet and that encourages us to build scalable. We provide java application development services across the world.

Anubhuti Mittal said...

TechGropse is a leading mobile app development company in Saudi Arabia that provides you the best services of android app development and iPhone app development. Our state-of-the-art plans and effective app development model help us to serve our customers efficiently. In recent years, we have emerged as a leading mobile app development company in Riyadh, Jeddah, and other regions of Saudi Arabia.