Beefy Boxes and Bandwidth Generously Provided by pair Networks
laziness, impatience, and hubris
 
PerlMonks  

Re: Net::Clacks Important data corruption bugfix released

by stevieb (Canon)
on Jul 08, 2021 at 07:54 UTC ( #11134786=note: print w/replies, xml ) Need Help??


in reply to Net::Clacks Important data corruption bugfix released

Three suggestions:

  • Write tests. You have no functional tests at all for anything, let alone regression tests for the issue you've fixed here
  • Break up your subs. A subroutine should do one thing, and do it well. When it gets longer than a short scroll, it's time to think about breaking it up into smaller pieces of functionality. The run() method is almost 1,000 lines. That's almost 1,000 lines too many. IMHO, a run() method should be concise in what it does. Its flow should be easy to follow and troubleshoot. Organize all of the direct functionality it does into logical code units (subs) with descriptive names. Dispatch tables can help you seriously clean up the if/else stuff going on
  • Write tests. Seriously. You've got a whole lot of conditional code in the module, and without tests, I can see a whole lot of problems arising out of even a simple change to the logic going on. Without tests, you might break 10 things without noticing before you even release the next version. Then to fix them without tests, you're bound to break another 10 unnoticed things

Replies are listed 'Best First'.
Re^2: Net::Clacks Important data corruption bugfix released
by cavac (Curate) on Jul 08, 2021 at 11:36 UTC

    Write tests. You have no functional tests at all for anything, let alone regression tests for the issue you've fixed here

    I find automated tests of limited use, and a "regression test" wouldn't have caught that specific issue since it's a timing issue/race condition. Instead, i run a couple of non-critical (private) servers that use Net::Clacks under real life conditions and exercise every part of the protocol. I usually upgrade those servers a few days before a new release to see if everything works as it should.

    I'm planning to clean up the code (up to a certain point). But the main function will most likely stay over 500 lines long, simply because it's a state machine and i like to be able to follow all the critical parts of the state flow without jumping in and out of subroutines in when reviewing the code. It's a personal preference thing.

    perl -e 'use Crypt::Digest::SHA256 qw[sha256_hex]; print substr(sha256_hex("the Answer To Life, The Universe And Everything"), 6, 2), "\n";'

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://11134786]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others surveying the Monastery: (2)
As of 2021-09-19 09:18 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found

    Notices?