On 4/7/21 1:19 PM, Jehan-Guillaume de Rorthais wrote:
> On Wed, 7 Apr 2021 12:51:55 -0400
> Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
>> On 2021-Apr-07, Jehan-Guillaume de Rorthais wrote:
>>
>>> When I'm creating a new node, I'm using the "pgaTester" factory class. It
>>> relies on PATH to check the major version using pg_config, then loads the
>>> appropriate class.
>> From a code cleanliness point of view, I agree that having separate
>> classes for each version is neater than what you call a forest of
>> conditionals. I'm not sure I like the way you instantiate the classes
>> in pgaTester though -- wouldn't it be saner to have PostgresNode::new
>> itself be in charge of deciding which class to bless the object as?
>> Since we're talking about modifying PostgresNode itself in order to
>> support this, it would make sense to do that.
> Yes, it would be much saner to make PostgresNode the factory class. Plus, some
> more logic could be injected there to either auto-detect the version (current
> behavior) or eg. use a given path to the binaries as Mark did in its patch.
Aren't you likely to end up duplicating substantial amounts of code,
though? I'm certainly not at the stage where I think the version-aware
code is creating too much clutter. The "forest of conditionals" seems
more like a small thicket.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
