-
-
Notifications
You must be signed in to change notification settings - Fork 388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Separate Configuration Objects from Configuration File Loading #233
Comments
That not so lovely thing is being refactored in it's own helper so that I can test it easier. Not sure to see how your chain loader would work in our case. Can you provide an example? What would your interface look like ? Thanks @chrisguitarguy |
Loader would be simple: interface Loader
{
// read the input and return a Configuration, returns `null` if the config
// is not supported
public function load($config);
} Chain loader would just look through things and find an appropriate loader final class ChainLoader implements Loader
{
/** @var Loader[] */
private $loaders = [];
public function __construct(array $loaders)
{
$this->loaders = $loaders;
}
public function load($config)
{
foreach ($this->loaders as $loader) {
if (null !== $confObj = $loader->load($config)) {
return $confObj;
}
}
return null;
}
} Each loader would do something like check the file path and the extension and decide if it can load the config. I'm not really sure what the Configuration interface would look like quite yet. Need to dig in a bit more to see how the configuration object is used. It's probably going to be pretty close to all the public methods that are present in configuration right now (minus the setters). Or maybe we just keep the config object as is for right now, removing some of the loading methods -- except for interface Configuration
{
public function getName();
public function getOutputWriter();
public function formatVersion($version);
public function getConnection();
public function getMigrationsTable();
public function getMigrations();
public function getVersion($version);
public function hasVersion($version);
public function hasVersionMigrated(Version $version);
public function getMigratedVersions();
public function getCurrentVersion();
public function getPrevVersion();
public function getNextVersion();
public function getRelativeVersion($version, $delta);
public function resolveVersionAlias($alias);
public function getNumberOfExecutedMigrations();
public function getNumberOfAvailableMigrations();
public function getMigrationsToExecute();
public function createMigrationsTable();
} The stuff like migrations directory and namespace only matter to the loader -- it can look in the directories and find the files, then just pass them to the configuration. |
@chrisguitarguy LoaderInterface and ChainLoader seems great. It's quite standard solution that's easy to extend and maintain. |
@chrisguitarguy @TomasVotruba Done in #422 Thanks |
Somewhat related to #204
It would make sense to pull out the file loading aspect of the configuration to a separate set of loader classes (and a chain loader that you just pass a filename too and avoid this lovely thing).
Then configurations objects themselves would just end up being a simple, immutable object hidden behind an interface that provide utilities like getting a versions.
Thoughts?
The text was updated successfully, but these errors were encountered: