Add Traitor class that doesn't perform validation checks

[djmattyg007]

The rationale behind this is that once you've successfully composed your
class once, it should continue to work, and shouldn't need to check
every interface and trait every time the code is run.

To assist in facilitating this, an interface has been added, and common
code has been abstracted into a parent class. Tests have also been
added.

[jmalloc]

Thanks for the PR, and especially for adding tests, but I'm afraid I can't see the benefit in adding this class. Please let me back that up with some rationale:

1. It's already possibly to skip duplicate checks by reusing the same Traitor instance:

$traitor = Traitor::create()
    ->implements_(SomeInterface::CLASS)
    ->use_(SomeTrait::CLASS);

$object1 = $traitor->instance(1, 2, 3);
$object2 = $traitor->instance(1, 2, 3);

2. This library is primarily intended for testing, where explicit error messages are much more valuable than the minimal performance improvement made by skipping input validation.

There is a small improvement that could be made to the existing implementation when reusing a traitor instance - the generated class name could be cached in Traitor::name(), though the cached name would need to be cleared each time any of the abstract_, use_, implements_ or extends_ methods are called.

Aaaaand... just to tack a shameless plug on the end - if you are using this library for testing traits, you might get some benefit from Phony, it's a fully featured stubbing/mocking library for PHP that directly supports creating test doubles from traits. I haven't been using Traitor in new code since I started using Phony.

[djmattyg007]

I was actually intending on using this package for something that isn't related to testing. My issue is that I feel it is unnecessary to perform all of those checks for a potentially large number of interfaces and traits on every request (and I would prefer to avoid caching serialized objects).

[djmattyg007]

I am open to completely changing the name of the class I added. I'm not very good at naming things.

[jmalloc]

I honestly think this is a premature optimisation, I doubt very much that this validation is a bottleneck in your application, and as a rule I heavily favour correctness over performance.

That said, I think a solution can be reached, I would just like it to be more explicit at the point where you use the traitor instance (rather than when it's created). This is important because a failure will result in a fatal error.

What I propose is this:

1. Keep the single Traitor class.
2. Add internal caching of the class name as mentioned above.
   • set $this->name to the generated name when eval'ing
   • set $this->name to null in use_, abstract_, extends_, implements_
   • only attempt to eval if $this->name is null, otherwise just return existing name
3. Delay the validation until just before the eval(), this has the added benefit of not autoloading the classes, interfaces and traits until they're actually needed.
4. Add a new method unsafeDeclare() that will perform the eval() and cache the name without performing the checks.

This makes it very obvious at the call site that you're performing an unsafe operation, and the method can be documented to describe the failure condition (the fatal error).

[djmattyg007]

I'm on board with only having a single class, and all of the other suggestions you made, but I'm not really a fan of calling the new method unsafeDeclare.

For me, removing these checks is very similar to how PHP7 ignores assert() calls during compilation. In development, they're essential, but once you're ready to deploy the code into production, they shouldn't be necessary anymore, and it shouldn't be unsafe to not perform the checks. I can't imagine a scenario where this isn't the case unless you're modifying the code before it reaches production.

[jmalloc]

I'm open to alternative names for sure. Though to my mind "it shouldn't be unsafe" means the same as "it is unsafe" :P I chose that name in reference to C#'s unsafe keyword.

For whatever it's worth, I really don't like letting my libraries produce fatal errors, for that reason I would never use assert() for this kind of validation either.

Anyway, now I just sound preachy! How about uncheckedDeclare() ?

[djmattyg007]

uncheckedDeclare() sounds fine to me.

Given I'm basically throwing my existing work away and starting again, would you like me to submit a separate PR? Also, would you like to keep the interface given it will just be a single class again?