use Perl

• Simplicity (Score:2)

  by petdance (2468)

  I think that Mark's intent was not to show how it would have been refactored completely, but rather to show the difference as it related to his original point. I'm certain that he left the code the way it was as a conscious choice.

  MJD is certainly adept at spotting red flags [plover.com].

  --
  

  --
  xoa
• The point. (Score:1)

  by Aristotle (5147)

  Anything that I overlooked?

  Yes: the point of his post. :-)

  It might have been useful if he had added a note to the effect that “this is still not very good, but it’s the first step to cleaning it up” – but a complete cleanup wasn’t what he set out to write about.
  • Re: (Score:2)

    by bart (450)

    Well, I beg to differ. In my draft for this post I had originally written (and next dropped it as overly harsh):

    When you touch up code, any junk that you leave behind in statements that you touch, becomes your junk.

    Leaving in the statement

    printf FILE "$LOCATION{$location}\,";

    now becomes his code.
• understanding beginners' code (Score:1)

  by mr_bean (3802)

  The other way of looking at the beginner's confused code would be to see it as an interaction between building on what you know and pulling rabbits out of a hat, and the hope that the two will eventually meet in the middle.
• Too harsh (Score:1)

  by jtrammell (6222)

  It's clear that MJD is addressing issue of the loop logic ("iterating over the keys of a hash"), and not what is done in the loop.
• I thought... (Score:1)

  by phillup (4419)

  But there are still more things wrong with the original code that he didn't discuss. For example:

      foreach $location_name (%LOCATION ) {
          $location_code = $LOCATION{$location_name};
          ...

  H*ll, if you want to loop over the keys of a hash, at least don't loop over both the keys and the values! Granted, in MJD's replacement code, the loop is gone, so this problem has disappeared too, but this is an major mistake that shouldn't just be skipped over.

  Actually, I thought the entire point of that part of the article was to point out that the user was using a loop to find the element that they could have simply addressed directly... and (I suppose) that is why he didn't address other "issues" in the code. They weren't the point.
  
  At least that was my interpretation...
  • Re: (Score:1)

    by jjore (6662)

    I thought it was interesting that MJD refactored the loop incorrectly. The original loop had the logic that if the key didn't exist in the hash, nothing would happen. MJD's refactoring caused it to be unconditional. That might be intended but it wasn't mentioned.
    
    the following bit would have been more accurate.

    if ( exists $hash{foo} ) {
        ++ $hash{foo}
    }