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.