http://www.perlmonks.org?node_id=1086993


in reply to Previous Line Matching Issues

Well, if it works, then so be it, but it would certainly flunk a code-review from me, and if you were on my team I would instruct you to rewrite it.   Let me explain why.

I have certain pre-conceived notions about how such an algorithm “should” be implemented, such that I am unnecessarily thrown-off by strategies which depart from these norms.   Specifically:

  1. I expect there to be only one place where the next-line of the file is read.   Your version contains at least two.   I am therefore immediately suspicious that this logic will skip lines.
  2. Cryptic variable names such as $cl and $nl, in a substantial piece of code like this, are not acceptable.
  3. This logic looks ahead, where I expect to see state-machine like logic which remembers its past.
  4. I can anticipate that this business requirement might soon call for looking-back some arbitrary-n lines, and I do not readily see how this logic would be readily adapted to do this.
  5. Overall complexity.   This approach is harder than it needs to be, and I doubt that it is accompanied by a rigorous test-suite sufficient to prove that it operates correctly in each and in every edge-case that it might encounter in production.

And please, don’t take this assessment personally.   It’s not about you.   This is engineering.

In any backward-looking loop like this one, the issue at hand is always how to correctly maintain the past-history, no matter what pathway is taken through the code.   The next if idioms at the top of the loop are red-flags to me:   you are not updating the history before you do next.   There are other red-flags that jump out at me ... and basically, I don’t want code in the source-base that has any red-flags that jump out at me.   I want to know that it is correct having proven that it is correct such that I never have to look at it again.

In closing, no, I did not exhaustively review the code as I would.   Instead, I saw enough of it to realize that it was not obvious, therefore it could not be obviously-correct, therefore it could be a future source of problems.   And so, that’s why I would instruct you to rewrite it and of course pay you to do so.