I'm close to releasing the first version of my library. And I want to dedicate some time about the ideas behind the refactoring.
I'm going to do that based on the collectQueryParameters function.
People that used PDO functions know how difficult it is to bind values securely to the query. The WHERE name = :name is the easy one, but then there is WHERE name IN (:name1,:name2). And the ultimate boss of the parameters is VALUES (:name1, :street1), (:name2, :street2).
With my library those queries will be;
WHERE ~Users:Name = :Users:NameWHERE ~Users:Name IN :Arr:NamesVALUES :MultiInsert:Users
The before and after
// old
1 function collectQueryParameters(
2 string $query,
3 QueryParametersCollection $parameters,
4 BaseNamespaceCollection|null $namespaces = null
5 ): array
6 {
7 $placeholders = queryToIdentifierOrPairCollection($query, "(:[A-Za-z1-9\\\]+:[A-Za-z1-9]+)", $namespaces);
8
9 if (count($placeholders) == 0) {
10 return [];
11 }
12
13 $placeholderReplacements = [];
14
15 foreach ($placeholders as $item) {
16 [$placeholder, $identifierOrPair] = $item;
17
18 if (is_array($identifierOrPair) && $identifierOrPair[0] == 'Array') {
19 $value = $parameters->getValue($placeholder);
20 if (is_array($value)) {
21 foreach ($value as $k => $v) {
22 $placeholderReplacements[$placeholder . '_' . $k] = $v;
23 }
24 }
25 } elseif ($identifierOrPair instanceof IdentifierInterface && $parameters->keyExists($identifierOrPair)) {
26 $placeholderReplacements[$placeholder] = $parameters->getValue($identifierOrPair);
27 }
28 }
29
30 return $placeholderReplacements;
31 }
// new
1 function collectQueryParameters(
2 string $query,
3 QueryParameterCollection $parameters,
4 BaseNamespaceCollection|null $namespaces = null,
5 ): PlaceholderIdentifierCollection|Error
6 {
7 $placeholders = queryToPlaceholderIdentifierCollection($query, getParameterRegex(), $namespaces);
8
9 if ($placeholders instanceof Error) {
10 return $placeholders;
11 }
12
13 $placeholderReplacements = new PlaceholderIdentifierCollection();
14
15 if ($placeholders->isEmpty()) {
16 return $placeholderReplacements;
17 }
18
19 $placeholdersInParameters = array_filter($placeholders->getAll(), fn($item) => $parameters->keyExists($item->identifier));
20
21 foreach ($placeholdersInParameters as $item) {
22 $phi = new PlaceholderIdentifier($item->placeholder, $item->identifier);
23 $value = $phi->getCustomValue($parameters->getValue($item->identifier));
24
25 if($value instanceof Error) {
26 return $value;
27 }
28
29 $phi->value = $value;
30
31 $placeholderReplacements->add($phi);
32 }
33
34 return $placeholderReplacements;
35 }
I will refer to these two functions as old and new below.
Initial observations
The new function is slightly longer than the old one. This is due to adding error handling.
When you look at the new function you don't see the hardcoded strings that were there in the old function. This is because they have been abstracted to provide users of the library, and myself, more options to use the function.
On line 7 you see in the old function a regex, while in the new function that has been replaced by a getParameterRegex() function.
The regex function checks for a PARAMETER_REGEX environment variable or defaults to regex you see in the old function.
On line 22 of the old function you see the guts of how it is made possible to use :Array:Names in the query. But there is more happening, and that deserves a new chapter.
Custom identifiers
For people who haven't read my previous posts. The library is called idable queries, it was composable queries. And the main goal is to have a common language, read functions, to work with different databases. While providing a single source of truth for the names used in the databases, that is where the identifiers come in.
In the old function you see several times the word pair. The reason is :Array:Names will be split up in Array and Names. The identifiers are enums so the library will check for the enum with the name Array and the case Names. In the old code when the name was not found the split array was added as is. And that is why in line 18 you see the code checks for Array as the first item.
In the new code the pairs are removed in favor of an identifier with a CustomIdentifier attribute.
#[CustomIdentifier('arrayTransformer')]
// Array is a keyword so you can't use it as an enum name.
enum Arr implements IdentifierInterface
{
case Names;
}
That is why you see in the new function on line 23 the getValue method call. With reflection the transformer function is fetched from the enum and executed.
This allows me in the database packages to set up transformer functions that library users can add to their identifiers. And they can also create their own.
The return types
In the old function you see on line 5 the return is an array. On the same line in the new function you see two types. PlaceholderIdentifierCollection is basically an array where the content is typed. I choose the random name PlaceholderIdentifier for the item type.
By using a type this allowed me to add methods that are making it easy to replace single placeholders with multiple. And sets up the parameters to add them to the query statement.
I already had a comment about the use of the Error type instead of throwing exceptions. So I want to address it here.
I think the main problem with throwing exceptions is that they don't appear in the function/method definition. So you need to look at the body to see it or use DocBlock comments.
It has been compared to an Either/Result monad, but to me it is more the PHP version of the Go function error.
// code stolen from the Go documentation. I'm sure they don't mind.
func Hello(name string) (string, error) {
// If no name was given, return an error with a message.
if name == "" {
return "", errors.New("empty name")
}
// If a name was received, return a value that embeds the name
// in a greeting message.
message := fmt.Sprintf("Hi, %v. Welcome!", name)
return message, nil
}
message, err := greetings.Hello("")
// If an error was returned, print it to the console and
// exit the program.
if err != nil {
log.Fatal(err)
}
I also prefer the flatter function structure of the error instance check over the try catch blocks.
And for me the most important reason is the chainablity of the functions. The pipe operator in PHP 8.5 is also one of the reasons I'm using functions instead of objects.
By using functions you are guided to write code without side effects. I didn't realize it until yesterday but the collectQueryParameters function is immutable because it creates a new PlaceholderIdentifierCollection based on the queryToPlaceholderIdentifierCollection it calls internally.
Next steps
The coming days I'm going to finish the refactoring and kick code with the nastiest tests I can think of, to see if there are still cracks in the code. And when I'm satisfied I will release the first version of the core and PDO packages.
The following milestone will be the Redis and MongoDB packages. And in the future I will add more database packages.
If you want a take-away from this post. I think it is, don't stare yourself blind on the code you have. Try to make it as good as it can get. It is time consuming, because the ideas don't come all at once. For that function I think I threw away code changes four or five times to come to this. But when you look at the code it seem to be minor changes. It are the ideas behind it that make the code valuable.
Top comments (0)