Stories
Slash Boxes
Comments
NOTE: use Perl; is on undef hiatus. You can read content, but you can't post it. More info will be forthcoming forthcomingly.

All the Perl that's Practical to Extract and Report

The Fine Print: The following comments are owned by whoever posted them. We are not responsible for them in any way.
 Full
 Abbreviated
 Hidden
More | Login | Reply
Loading... please wait.
  • by nik (3476) <nikc@cpan.org> on 2007.10.21 21:12 (#58500) Journal
    I've had to fix enough code where the original programmer's intent wasn't clear, or is lost in the mists of time.

    In the absence of some explicit documentation that says the example function doesn't return anything, the change you show seems reasonable.

    And if you have explicit documentation that says the example function doesn't return anything, the change also seems reasonable -- it guarantees that someone using your interface can't ignore the documentation, depend on undocumented functionality, and force you to support it from then on.
    • In the absence of some explicit documentation that says the example function doesn't return anything, the change you show seems reasonable.

      I disagree. If it doesn't say it returns nothing, then you should assume it returns something.

      In complete absence of info on what it returns, the code should thus become:

          sub prepare_report {
            my ($self, $file) = @_;
            return $self->analyze($file)->report;
          }

      • Mindlessly putting it one place is just as wrong as mindlessly putting it in another. If you don’t know where it goes, you don’t put it anywhere – you go and look at the docs and the rest of the codebase. If that’s not in the cards, for whatever reason, then you leave damn well alone – even if that means Perl::Critic continues moaning about it. Because it should. Filling in the paperwork just to make it happy without knowing what you’re doing is just self-deception.

    • Well, first of all, there *were* explicit docs that said that text was returned.

      Beyond that, though, this change caused the entire module to become completely useless, which means that it wasn't desk checked. Why not? Because the module wasn't undergoing any actual maintenance. In the course of other work, someone saw Perl::Critic complain about "no explicit return" and added one, without further testing.

      Sure, automated testing would've caught this, but if there are no automated tests, you don't make cha
      --
      rjbs