Note that there are some explanatory texts on larger screens.

plurals
  1. PO
    text
    copied!<p>Since this is code review, let's go one by one:</p> <pre><code>#!/use/bin/perl </code></pre> <p>That shebang line is most likely a typo. It should probably be</p> <pre><code>#!/usr/bin/perl </code></pre> <p>or whatever <code>which perl</code> returns on your system.</p> <pre><code>use strict; use warnings; </code></pre> <p>Good.</p> <pre><code>open (FILE, "/home/user/Desktop/infile.phy") || die "cant open file\n"; </code></pre> <p>No need for package global filehandles when you can use lexical filehandles. The 3-argument form of <code>open</code> is preferable these days. Also, the error message should indicate the file which you could not open:</p> <pre><code>my $filename = '/home/user/Desktop/infile.phy'; open my $input, '&lt;', $filename or die "Cannot open '$filename' for reading: $!"; my @body = &lt;FILE&gt;; </code></pre> <p>You are slurping the file into an array. That is completely unnecessary in this case.</p> <pre><code>my $count = 0; my $string = ''; </code></pre> <p>Declare and initialize (if necessary) any variables in the smallest possible scope.</p> <pre><code>my $count; </code></pre> <p>The variable <code>$string</code> is not used anywhere else in your code.</p> <pre><code>foreach $_(@body){ </code></pre> <p>This is silly. <code>for</code> uses $_ if no loop variable is specified. It is easier to keep things straight if you instead specify a lexical loop variable.</p> <pre><code>for my $line ( @body ) { </code></pre> <p>However, I do not think you should slurp the file.</p> <pre><code> if ($_ =~ m/[X]/){ </code></pre> <p>That results in a successful match if the line contains an X. So, it is equivalent to <code>/X/</code>. However, that will not tell you the word that contained the 'X'. For that, you need to decide what a word is and do your matching at the word level.</p> <p>With all that in mind, consider the following script. I have made a simplifying assumption regarding what I consider to be a word. You should be able to build on this to satisfy all the requirements:</p> <pre><code>#!/usr/bin/perl use strict; use warnings; my $filename = "$ENV{TEMP}/test.txt"; open my $input, '&lt;', $filename or die "Cannot open '$filename' for reading: $!"; my $count; while ( my $line = &lt;$input&gt; ) { my @words = grep { /X/ } split /\b/, $line; $count += @words; print join(', ', @words), "\n"; } print "$count\n"; __END__ </code></pre> <p><strong>UPDATE:</strong> If you do not care about finding the words within each line that have one or more X characters, the while loop would be simplified:</p> <pre><code>while ( &lt;$input&gt; ) { $count += (my @matches = /(X)/g ); print if @matches; } </code></pre> <p>by using $_. That, however, is probably inefficient (given that we are saving each matched X character). In this case, <code>tr</code> works best:</p> <pre><code>my ($count, $n); $n = tr/X// and $count += $n and print while &lt;$input&gt;; </code></pre>
 

Querying!

 
Guidance

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload