SimpleNeo4j\Client candidate for builder pattern

[callebstrom]

Hello,

Typodeaf from devRant here.

Looking at the test for the Client I saw a possibility to implement a builder for the Client.

I think this would contribute to readability, especially since I have a feeling the args list will grow.

Wdyt?

I'm imagining defining different Clients like this:

$builderOne = new SimpleNeo4j\ClientBuilder('some_host');
$client = $builderOne->username('some_user')->password('pw')->build();

$builderTwo = new SimpleNeo4j\ClientBuilder('some_host');
$client = $builderTwo->guzzle($http_client)->build();

or

$client = SimpleNeo4j\Client::createBuilder('some_host')->username('some_user')->password('pw')->build();

$client = SimpleNeo4j\Client::createBuilder('some_host')->guzzle($http_client)->build();

[DFoxinator]

Thanks for the suggestion!

I was circling back and starting to make tickets for this tonight. I think you're definitely right in that the args list for the constructor for client will get out of hand.

I think the best approach might be to do it Guzzle-style, where anything with over a few parameters takes a config array. Builder is pretty cool, but I think for configuring a database client it's easier to be able to pass in an array of config options that you can store in your app config like many apps do.

Please let me know your thoughts.

[callebstrom]

Yes that is a good point. A config array is probably better, especially if you need re-instantiate. What I prefer about the builder is that it allows for the IDE to provide hints about the available configuration parameters without having to look into the constructor manually and that you don't need any constructor code to ensure the required configuration is set.

But now that I think about it you can also have a default array parameter with all the available configuration keys set to null.

I think both of them will serve equally well for this purpose and, as you mentioned, sticking to the Guzzle standard probably makes more sense for the library consumers.