nelmio/alice — pull request #97 by tshelburne (2014-03-18 07:31:21+00:00)
Title: Library refactor into classes

**To begin, the existing public interface and fundamental test suite have not changed - this has primarily been cleanup and organization.**

- [ Justification ](#justification)
- [ Improvements and Benefits ](#improvements-and-benefits)
- [ `Base#parseFile` ](#added-protected-baseparsefile)
- [ `Collection` class ](#collection-class-introduced)
- [ `Fixture` class ](#fixture-class-introduced)
- [ `PropertyDefinition` class ](#propertydefinition-class-introduced)
- [ Utility classes ](#utility-methods-moved-into-utility-classes)
- [ `Builder` class ](#builder-introduced)
- [ `Instantiator` class ](#instantiator-introduced)
- [ `Processor` class ](#processor-introduced)
- [ `Populator` class ](#populator-introduced)
- [ Disclosure ](#disclosure)
- [ Next Steps ](#next-steps)

## Justification ##

I’ve been working in a project which had no seeding and no test fixtures, and the test infrastructure was beginning to balloon out of control. Alice’s interface looked wonderful, but the framework being used has some fundamental disagreements with the way Alice assumes objects should be built.

I tend to side with Alice, but my hands are tied - when I tried to add functionality through creating my own extensions, the methods available to override on `Base` were very long, very complex, and often private. As a consequence, I found myself copying and pasting large pieces of the `Base` loader, duplicating methods, and generally breaking a lot of rules.

My main goal was to separate concerns and break long code sections into smaller, simpler chunks. Although the changes are substantial, **the existing public interface and tests have not changed**, and I intend to add even more granular tests of the individual components as time permits. This was a pretty fast refactor (about 10 days), and I’m needing to get back into the project for which I’m using Alice.

Every commit represents a step in the process of refactoring, and all tests *should* pass at every commit. The history should make my thinking and my approach very clear as I worked to simplify things.

## Improvements and Benefits ##

### Added protected `Base#parseFile` ###
- Allows extensions to only overwrite the file parsing mechanism for custom loaders (XML, JSON, etc.)
- Simplified the `Yaml` loader

### `Collection` class introduced ###
- This may necessitate moving “doctrine/common” to require (see [disclosure](#disclosure))
- Puts all references into a list of available instances
- Saves objects for successive builds using the same loader
- Codified access to the currently available instances
- Extension of Doctrine `ArrayCollection`, so it has a well-defined interface

### `Fixture` class introduced ###
- Represents a fixture in code with stricter rules, rather than using `array`s and `list()`
- Allows for simpler access to things like `__construct`, `__set`, etc. without needing to `unset`
- Allows for run-time type checking in method arguments, and simplified method interfaces
- Greatly simplified implementation of instantiating by constructor method

### `PropertyDefinition` class introduced ###
- Represents a property definition from a fixture in code
- Simplified the implementation of property value processing substantially

### Utility methods moved into utility classes ###
- `#parseFlags` and `#checkTypeHints`
- Substantially simplified the look of the `Base` loader, and helped share the functions in other classes

### `Builder` introduced ###
- A strategy pattern to generate fixtures from raw array data allows for easier addition of new build mechanisms (currently `SimpleName`, `ListName`, and `RangeName`)
- With the addition of `Base#addBuilder`, it is trivial to add custom build rules

### `Instantiator` introduced ###
- A strategy pattern to generate instances of class designated by a fixture, which allows for easier addition of new instantiator mechanisms (currently `EmptyConstructor`, `ReflectionWithConstructor`, `ReflectionWithoutConstructor`, and `Unserialize`)
- With the addition of `Base#addInstantiator`, it is trivial to add custom instantiators in the event your project has more specific domain rules

### `Processor` introduced ###
- A chain of responsibility pattern to manipulate a property value to the point of being put on the instance itself, which allows for easier addition of new processing mechanisms (currently `ArrayValue`, `Conditional`, `Faker`, `Reference`, and `UnescapeAt`)
- With the addition of `Base#addProcessor`, it is be trivial to add custom processors in the event your project has very specific concerns or “DSL”s that imply more than a custom provider
- The method for handling current values was moved into the `Processor#process` function itself, removing dependence on remembering to set a global current value every time it is called - ultimately, this should probably move toward having a reference to a "currentFixture" rather than "valueForCurrent"

### `Populator` introduced ###
- A strategy pattern to set properties described by each fixture, which allows for easier addition of new population mechanism (currently `ArrayAdd`, `ArrayDirect`, `Custom`, `Direct`, and `Property`)
- Some objects (particularly ActiveRecord) rely on `__call`, `__get`, `__set`, etc. to handle attributes - with the addition of `Base#addPopulator`, this extension is trivial to implement (**this was my main reason for my entire update**)

## Disclosure ##
- I can understand wanting to remove extension of Doctrine `ArrayCollection` so as not to have another dependency. Happy to do it if necessary.
- I forgot to look at the basic style of commit messages before doing a lot of my earlier work - sorry for the discontinuity

## Next Steps ##
- Simplify and DRY up the `Faker` processor class to use functions instead of `eval()` and to pull out the reference replacement part
- Change `Base#parseFile` method to run a series of parsers to painlessly handle a limitless number of filetypes
- Clean up the `Fixtures` class - I haven’t touched this yet because I’m not really using it
- If `ArrayCollection` use is deemed acceptable, start using it throughout to clean up array use