Library scope

[happyr]

Library scope

[cakper]

Hey, @Nyholm sorry I was inactive recently :) Will reply today on the evening :)

[Nyholm]

No problem. I knew that you had your events and presentation to prepare.

[cakper]

I was thinking about it, and I'm not really sure what will be the best approach.

I think if we want to deliver complete library we would need to implement all the other specs, that will also allow to delete, update, select only specific things, partial selecting, grouping, make query distinct etc

It will be bigger thing, but maybe it's worth it to have complete solution? Also an option is to release stable version 1.0 with what we have at the moment, and then work on the things I've mentioned.

[Nyholm]

I want to deliver a complete library.

Let's fix #21 and #24 and then release the first version 0.1 Make sure it matures before we release the first stable version 1.0. Sounds all right?

[Nyholm]

We also need to make a decision if we are happy with the source structure and the naming of the directories and classes. It is much harder to change after we released a stable version.

Example:

// Is this the folder or class?
use Happyr\Doctrine\Specification\Spec;

class Foo 
{
  function bar() {
    $a = Spec::eq('field', 'value');
    $b = new Spec\Equals('field', 'value');
  }
}

It might be a problem for some operating system, PHP versions or future PHP versions that use Happyr\Doctrine\Specification\Spec is be referring both a class and a directory.

[cakper]

Yeah, I agree with structure and naming. What if we just put all sources under Specification?
Or maybe group with Specification\Logic, Specification\Comparison?
So that we will be able to have generic stuff under main namespace, and also Spec static helper.

[Nyholm]

The problem with grouping is that it leads to many imports...

use Happyr\Doctrine\Specification\Logic\AndX;
use Happyr\Doctrine\Specification\Comparison\Equals;

class Foo 
{
  function bar() {
    $spec = new AndX(new Equals('field', 'value'), new Equals('field2', 'value2'));
  }
}

use Happyr\Doctrine\Specification as S;

class Foo 
{
  function bar() {
    $spec = new S\Logic\AndX(new S\Comparison\Equals('field', 'value'), new S\Comparison\Equals('field2', 'value2'));
  }
}

I think we should have all Specification under one common namespace.. Like:

use Happyr\Doctrine\Specification\Whatever as S;

class Foo 
{
  function bar() {
    $spec = new S\AndX(new S\Equals('field', 'value'), new S\Equals('field2', 'value2'));
  }
}

I do think that the EntitySpecificationRepository should have a different namespace from the Specifications.

[Nyholm]

Having all sources under the Happyr\Doctrine\Specification namespace feels okej,... but weird at the same time. We will probably have 15-20 classes in that namespace soon...

This is how Doctrine does it: https://github.com/doctrine/doctrine2/tree/master/lib/Doctrine/ORM/Query

[Nyholm]

At a second thought... I like the grouping. You should use the factory class if you want to reduce your imports.

I agree with @cakper on the following namespaces:

• Happyr\Doctrine\Specification the factory class and EntitySpecificationRepository.
• Happyr\Doctrine\Specification\Logic, Happyr\Doctrine\Specification\Comparison, Happyr\Doctrine\Specification\SomeForJoinAndLimit etc...

[cakper]

Ok, so my proposition is to deal with all remaining tasks before release, think abut it a bit more, and then once everything is done we will be able rename it painlessly :)

[Nyholm]

I suggest the following grouping. But where does Join belong?

Happyr\Doctrine\Specification\Logic

• AndX
• LogicX
• Not
• OrX

Happyr\Doctrine\Specification\Comparison

• Comparison
• Equals
• GreaterOrEqualThan
• GreaterThan
• In (not sure)
• IsNotNull
• IsNull
• LessOrEqualThan
• LessThan
• Like

Happyr\Doctrine\Specification\Query

(Not sure about the name of this namespace)

• AsArray
• Join
• Limit
• OrderBy

Happyr\Doctrine\Specification

• BaseSpecification
• EntitySpecificationRepository
• Spec (Maybe renamed to SpecFactory)
• Specification (Maybe renamed to SpecificationInterface)

Other

• Join

[cakper]

I think Query namespace is quite ok - it reads good - S\Query\Join S\Query\Limit and so on, so join belongs there.

RE "Specification (Maybe renamed to SpecificationInterface)" I'm not convinced by that. I do not consider adding Interface suffix a best practice, and we actually don't do that on projects. Some reading on it: http://verraes.net/2013/09/sensible-interfaces/

RE "Spec (Maybe renamed to SpecFactory)" I think it's good to have names that contain function of the class and I think too that classes should be easy to use, so because primary goal of this class is to provide convenient way of using Specifications name Spec is better because it is just a helper class :)

[Nyholm]

@cakper, I really value your arguments. 👍 The reson I brought these issues up is because I wanted this discussion. You have convinced me on all the bullets above.

I'll make a PR that closes this issue.

[cakper]

@Nyholm I was thinking about main namespace - what do you think about shortening it a bit and instead having Happyr\Doctrine\Specification we will just rename it to Happyr\DoctrineSpecification.

This one additional level doesn't give us any benefits (just adds complexity to imports), and we are not extending doctrine core either.

[Nyholm]

Agreed. I'll update #34

[Nyholm]

There @cakper. I've updated the namespace to Happyr\DoctrineSpecification. I would like to have your input on #34.