Skip to content
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

Add Config Collection #146

Merged
merged 1 commit into from
May 23, 2017
Merged

Conversation

jakejohns
Copy link
Member

I've been breaking up my ContainerConfig classes into smaller little discrete chunks for organizational and reuse purposes. What I often end up with are configs like Service\Config or whatever, that then just delegates to a bunch of other configs.

I end up basically duplicating the functionality in the ContainerBuilder (instantiating classes, checking interface, calling define, calling modify), so I thought perhaps it might be useful to pull it out into its own class, so I can just extend that instead.

I end up with something like this:

<?php
//@codingStandardsIgnoreFile

// src/
// ├── Config.php
// ├── Data
// │   └── Config.php
// ├── Service
// │   ├── Config.php
// │   ├── Posts
// │   │   └── Config.php
// │   └── Users
// │       └── Config.php
// ├── View
// │   ├── Config.php
// │   └── Helpers
// │       └── Config.php
// └── Web
//     ├── Config
//     │   ├── Middleware.php
//     │   └── Routes.php
//     └── Config.php


$builder->newConfiguredInstance([App\Config::class]);


// NS My\App
class Config extends ConfigCollection
{
    protected $packages = [
        Data\Config::class,
        Service\Config::class,
        View\Config::class,
        Web\Config::class,
    ];

    public function __construct()
    {
        parent::__construct($this->packages);
    }
}


// NS My\App\Web
class Config extends ConfigCollection
{
    protected $packages = [
        Config\Routes::class,
        Config\Middleware::class,
    ];

    public function __construct()
    {
        parent::__construct($this->packages);
    }
}

// NS My\App\Service
class Config extends ConfigCollection
{
    protected $packages = [
        Posts\Config::class,
        Users\Config::class
    ];

    public function __construct()
    {
        parent::__construct($this->packages);
    }
}

Thoughts?

@djmattyg007
Copy link
Contributor

What is the purpose of having your individual config classes extend the ConfigCollection class? The semantics of inheritance mean that you're saying "Config" is a "ConfigCollection", which doesn't really make sense.

@jakejohns
Copy link
Member Author

@djmattyg007 you might be right... but I guess why is my App\Config here not functionally a collection of my Data, Service, Web, and View configs?

My impulse here comes from the fact that I've found myself doing things like this from time to time, especially in larger projects, as a way of breaking configs up into smaller discrete manageable chunks. This way I can avoid a single giant config file. In addition to not wanting a single giant config for an app because its unwieldy, little chunks make it easier to load parts for various contexts (eg. cli vs web, etc)

@pmjones
Copy link
Member

pmjones commented May 23, 2017

And no changes to the tests? Nice. I can see @djmattyg007 's point, but even so, my related nitpicks are not big enough to be a blocker. Thanks!

@pmjones pmjones merged commit 8683f5a into auraphp:3.x May 23, 2017
@jakejohns
Copy link
Member Author

Cool. Although I'd be curious about your "related nitpicks".

@jakejohns jakejohns mentioned this pull request Jul 21, 2017
jakejohns added a commit to jakejohns/Aura.Di that referenced this pull request Aug 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants