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)
