On 2021-Apr-24, Andrew Dunstan wrote: > > I would like to undertake some housekeeping on PostgresNode.pm. > > 1. OO modules in perl typically don't export anything. We should remove > the export settings. That would mean that clients would have to call > "PostgresNode->get_new_node()" (but see item 2) and > "PostgresNode::get_free_port()" instead of the unadorned calls they use now.
+1 > 2. There are two constructors, new() and get_new_node(). AFAICT nothing > in our tests uses new(), and they almost certainly shouldn't anyway. > get_new_node() calls new() to do some work, and I'd like to merge these > two. The name of a constructor in perl is conventionally "new" as it is > in many other OO languages, although in perl this can't apply where a > class provides more than one constructor. Still, if we're merging them > then the preference would be to call the merged function "new". Since > we'd proposing to modify the calls anyway (see item 1) this shouldn't > impose a huge extra workload. +1 on "new". I think we weren't 100% clear on where we wanted it to go initially, but it's now clear that get_new_node() is the constructor, and that new() is merely a helper. So let's rename them in a sane way. > Another item that needs looking at is the consistent use of Carp. > PostgresNode, TestLib and RecursiveCopy all use the Carp module, but > contain numerous calls to "die" where they should probably have calls to > "croak" or "confess". I wonder if it would make sense to think of PostgresNode as a feeder of sorts to Test::More and the like, so make it use diag(), note(), explain(). -- Álvaro Herrera Valdivia, Chile "If you have nothing to say, maybe you need just the right tool to help you not say it." (New York Times, about Microsoft PowerPoint)
