The lack of any systematic indenting and spacing of the code is a very serious flaw. Trying to read your code is like trying to read a book which has no chapters, paragraphs or even punctuation marks! This is so serious that this code would not get more than a "D" in a college class even if it did work. I'm not saying that to be mean or condescending, you may not be aware of just how important this.

There is more than one way to do the indenting. As a beginner at this, I would suggest the "line up the braces method", like the example below. Use 3 or 4 extra spaces between levels. Alternately, you can use the diagonal brace method - its just as valid, but I suggest trying this method for awhile and see how you like it. Judiciously applied whitespace is one the most important things that you can do to improve readability!

while (condition) { .... if () { .... } .... }
Another systemic flaw is that your code doesn't have locality of functionality - related things are in widely separated areas. Some examples... I would open all of the files at the beginning of the code. BTW, I suspect some better names than "OUTPUT3" or "OUTPUT2"can be thought of... like maybe CSV or SORTED? I see print OUTFILE3 $lookup;. Don't defer output til later. Do right after you calculate it! I don't think that you even need this $lookup variable...print the lines as you go. print CSV "$ID,$nhs,$postcode,$dob,$sex\n"; Likewise, print OUTFILE2 @sorted;. Put that right after the sort.

A whole bunch of these chomp; statements appear to unnecessary. Try to factor out common code. Perhaps;

while (my $line = <INFILE>) { $line =~ s/^\s*//; # remove leading whitespace $line =~ s/\s*$//; # remove ending whitespace (includes \n) # chomp is not necessary ... }
Anyway think about the suggestions you've gotten and take another stab at this. Get it running with "use strict; use warnings;" and clean-up formatting.

In reply to Re: Code Critique by Marshall
in thread Code Critique by rhiridflaidd

Title:
Use:  <p> text here (a paragraph) </p>
and:  <code> code here </code>
to format your post, it's "PerlMonks-approved HTML":



  • Posts are HTML formatted. Put <p> </p> tags around your paragraphs. Put <code> </code> tags around your code and data!
  • Titles consisting of a single word are discouraged, and in most cases are disallowed outright.
  • Read Where should I post X? if you're not absolutely sure you're posting in the right place.
  • Please read these before you post! —
  • Posts may use any of the Perl Monks Approved HTML tags:
    a, abbr, b, big, blockquote, br, caption, center, col, colgroup, dd, del, details, div, dl, dt, em, font, h1, h2, h3, h4, h5, h6, hr, i, ins, li, ol, p, pre, readmore, small, span, spoiler, strike, strong, sub, summary, sup, table, tbody, td, tfoot, th, thead, tr, tt, u, ul, wbr
  • You may need to use entities for some characters, as follows. (Exception: Within code tags, you can put the characters literally.)
            For:     Use:
    & &amp;
    < &lt;
    > &gt;
    [ &#91;
    ] &#93;
  • Link using PerlMonks shortcuts! What shortcuts can I use for linking?
  • See Writeup Formatting Tips and other pages linked from there for more info.