On 4/7/21 1:19 PM, Jehan-Guillaume de Rorthais wrote: > On Wed, 7 Apr 2021 12:51:55 -0400 > Alvaro Herrera <alvhe...@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
