Why does my program crash after one line of input?

Question

I am writing a simple program which capitalizes each word in a sentence. It gets a multi-line input. I then loop through the input lines, split each word in the line, capitalize it and then join the line again. This works fine if the input is one sentence, but as soon as I input two lines my program crashes (and if I wait too long my computer freezes.)

Here is my code

@input = <STDIN>;
foreach(@input)
{   
        #reset @words
    @words= ();

    #readability
    $lines =$_;

    #split sentence
    @words = split( / /, $lines );
    #capitalize each word 
    foreach(@words){
            $words[$k] = ucfirst;
            $k++;
    }

    #join sentences again
    $lines = join(' ', @words);

    #create output line
    $output[$i]=$lines;
    $i++;
}

#print the result
print "\nResult:\n";
foreach(@output){
        print $output[$j],"\n";
        $j++;
}

Could someone please tell me why it crashes?

Think about where `$k` starts on the second line … but seriously, declaring all your variables via `my` and forcing yourself to do that via `use strict` would be far better than kludges like “reset @words”. — amon, Mar 27 '14 at 21:40
This can be done a lot simpler using `s/(\W+)/\u$1/g`. Ditto on [`Why use strict and warnings?`](http://stackoverflow.com/questions/8023959/why-use-strict-and-warnings) — Miller, Mar 27 '14 at 21:41
Thank you, I'm a beginner, I am sorry for not knowing about strict and warnings. My code works now, thank you. I wasn't allowed to use regex, I had to use ucfirst. — Simon H, Mar 27 '14 at 21:51
In student exercises they typically make you use the new material :) — brian d foy, Apr 09 '14 at 20:39

score 0 · Answer 1 · answered Mar 27 '14 at 21:56

use strict (and be told about not properly handled variables like your indices)
use for var (array) to get a usable item without an index (Perl isn't Javascript)
What isn't there can't be wrong (e.g. push instead of index)

In code:

use strict; # always!

my @input = <STDIN>; # the loop need in- and output
my @output = ();

for my $line (@input) # for makes readability *and* terseness easy
{
    chomp $line; # get rid of eol

    #split sentence
    my @words = split( / /, $line );

    #capitalize each word
    for my $word (@words){  # no danger of mishandling indices
        $word = ucfirst($word);
    }

    #join sentences again
    $line = join(' ', @words);

    #create output line
    push @output, $line;
}

#print the result
print "\nResult:\n";
for my $line (@output){
    print $line, "\n";
}

score 0 · Answer 2 · answered Mar 27 '14 at 22:22

The problem is that you are using global variables throughout, so they are keeping their values across iterations of the loop. You have reset @words to an empty list even though you didn't need to - it is overwritten when you assign the result of split to it - but $k is increasing endlessly.

$k is initially set to undef which evaluates as zero, so for the first sentence everything is fine. But you leave $k set to the number of elements in @words so it starts from there instead of from zero for the next sentence. Your loop over @words becomes endless because you are assigning to (and so creating) $words[$k] so the array is getting longer as fast as you are looping through it.

The same problem applies to $i and $j, but execution never gets as far as reusing those.

Alshtough this was the only way of working in Perl 4, over twenty years ago, Perl 5 has made programming very much nicer to write and debug. You can now declare variables with my, and you can use strict which (among other things) insists that every variable you use must be declared, otherwise your program won't compile. There is also use warnings which is just as invaluable. In this case it would have warned you that you were using an undefined variable $k etc. to index the arrays.

If I apply use strict and use warnings, declare all of your variables and initialise the counters to zero then I get a working program. It's still not very elegant, and there are much better ways of doing it, but the error has gone away.

use strict;
use warnings;

my @input = <STDIN>;
my @output;
my $i = 0;

foreach (@input) {

  # readability
  my $lines = $_;

  # split sentence
  my @words = split ' ', $lines;

  # capitalize each word
  my $k = 0;
  foreach (@words) {
    $words[$k] = ucfirst;
    $k++;
  }

  # join sentences again
  $lines = join ' ', @words;

  #create output line
  $output[$i] = $lines;
  $i++;
}

print "\nResult:\n";
my $j = 0;
foreach (@output) {
  print $output[$j], "\n";
  $j++;
}

Why does my program crash after one line of input?

2 Answers2