Skip to content

Configuration class maintainability #34

Open
@redthor

Description

@redthor

Scrutinizer gives Configuration class an F:
https://scrutinizer-ci.com/g/doesntmattr/mongodb-migrations/code-structure/master/class/AntiMattr%5CMongoDB%5CMigrations%5CConfiguration%5CConfiguration

It is not hard to see why.

There is this confusing inheritance model:

       Configuration (concrete)
            |
            v
   AbstractFileConfiguration (abstract)
      |                |
      v                v
XmlConfiguration YamlConfiguration  (concrete)

And there are a bunch of mutators on Configuration just so that the bundle can overwrite settings here: https://github.com/doesntmattr/mongodb-migrations-bundle/blob/master/Command/CommandHelper.php#L38

Suggest we apply:

  • Composition over inheritance;
  • Apply the single responsibility pattern;
  • Immutable configuration;
Class Responsibility
ConfigurationBuilder Create an immutable Configuration class
ConfigurationLoader Picks the way the configuration is loaded: `yaml
YamlLoader Reads YAML file. May not be required if we are just using Syfmony Yaml classes
XmlLoader Reads XML file. May not be required if it is too simple
ContainerLoader Gets container parameters
VersionCollection Container for the versions. Has the responsibility for looking up Versions etc

These are just ideas. Would need to look more into how the Configuration class is used.

Note that improvements here could also improve the Doctrine migrations library https://github.com/doctrine/migrations because it follows the same structure: https://scrutinizer-ci.com/g/doctrine/migrations/code-structure/master/class/Doctrine%5CDBAL%5CMigrations%5CConfiguration%5CConfiguration

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions