dg / dibi

Dibi - smart database abstraction layer
https://dibiphp.com
Other
487 stars 136 forks source link

Make Expression into an interface #385

Closed dakujem closed 3 years ago

dakujem commented 3 years ago

I would like to suggest to make Dibi\Expression class into an interface, as there is currently no option to use anything else than inherit Expression and am afraid it will become final in the future without means to extend/replace... Also, the use of private $values is a hindrance.

Currently, it's only used inside the Translator class with instanceof operator, which si an ideal situation to turn it into an interface.

IExpression or Expressable.

I'll gladly provide a PR.

milo commented 3 years ago

I don't think that expression should be an interface as is. Expression represents Dibi specific syntax. If so, an interface should say that object can be converted into Dibi Expression e.g. via method function toDibiExpression(): Expression.

dakujem commented 3 years ago

I don't see how that is different. Your object would have to return an instance of "dibi specific syntax" anyway and it would achieve the same. But I would be comfortable with that too, anything is better than inheriting from internal objects, that tend to become final, private and such...

dg commented 3 years ago

Why do you specifically need it?

dakujem commented 3 years ago

I'm writing a convertor that converts key-value arrays to conditions, agnostic of the storage backend.

For SQL, I'm trying to use Dibi. One of the objects I internally use is a "self-unpackable" condition used on both Fluent like this $expression($fluent) as a decorator, and in "regular sql" with %ex modifier.

With Dibi as it is, I need to either extend Expression or use a workaround to support both, neither being an elegant solution.

dg commented 3 years ago

And why can't the converter create a Dibi\Expression object?

dakujem commented 3 years ago

At the time of writing, I had a syntax tree composed of backend-agnostic nodes. I intended for these nodes to be instances of Expression, but I since scraped the idea and now use a tree traversal algo to converted the structure to a structure of nested Expressions. The downside of this is one more traversal.

This issue was meant to future-proof extensibility options with similar use cases, but I'm fine with the current state for now.