> On 17 Mar 2023, at 14:48, Andrew Dunstan <and...@dunslane.net> wrote: > On 2023-03-17 Fr 05:48, Daniel Gustafsson wrote: >>> On 15 Mar 2023, at 02:03, Andres Freund <and...@anarazel.de> >>> wrote: >>> >>>> Returning a hash seems like a worse option since it will complicate >>>> callsites >>>> which only want to know success/failure. >>>> >>> Yea. Perhaps it's worth having a separate function for this? ->query_rc() >>> or such? >>> >> If we are returning a hash then I agree it should be a separate function. >> Maybe Andrew has input on which is the most Perl way of doing this. > > I think the perlish way is use the `wantarray` function. Perl knows if you're > expecting a scalar return value or a list (which includes a hash). > > return wantarray ? $retval : (list or hash);
Aha, TIL. That seems like precisely what we want. > A common perl idiom is to start private routine names with an underscore. so > I'd rename wait_connect to _wait_connect; There are quite a few routines documented as internal in Cluster.pm which don't start with an underscore. Should we change them as well? I'm happy to prepare a separate patch to address that if we want that. > Why is $restart_before_query a package/class level value instead of an > instance value? And why can we only ever set it to 1 but not back again? > Maybe we don't want to, but it looks odd. It was mostly a POC to show what I meant with the functionality. I think there should be a way to turn it off (set it to zero) even though I doubt it will be used much. > If we are going to keep this as a separate package, then we should put some > code in the constructor to prevent it being called from elsewhere than the > Cluster package. e.g. > > # this constructor should only be called from PostgreSQL::Test::Cluster > my ($package, $file, $line) = caller; > > die "Forbidden caller of constructor: package: $package, file: > $file:$line" > unless $package eq 'PostgreSQL::Test::Cluster'; I don't have strong feelings about where to place this, but Cluster.pm is already quite long so I see a small upside to keeping it separate to not make that worse. -- Daniel Gustafsson
