Class to manage config data to allow DI

I'm writing my own framework (purely for learning purposes, SPL, OOP, patterns, etc) and have written a class to manage config data throughout the framework (not front end/view, just the core framework).

Example of config data:

The framework requires a specific PHP version to run correctly, and that "required PHP version" is stored in config settings. That config data is needed in a class which checks if the running PHP version is not lower than the required PHP version.

I've read about various options for configs (for tens of hours over weeks), and the main ones seem to be:

Constants; ini files; an array in an included file; static classes.

They all have pros and cons, but the main con is they push things into global, which I'm really trying to avoid.

They also don't lend themselves to dependency injection, which I'm really trying to achieve.

I've made my own class for managing this data, and would like it reviewed, please.

Class Description

The config class is instantiated in the bootloader file before any classes which need it, so the object variable is ready to be used in any class to call the method.

Note: I'll be changing how this class is instantiated when I get around to Dependency Injection Container, so please review based on that, and ignore that the object variable for this class is global - it eventually won't be.

The config class' method getConfig() is used in the __construct (or getter method etc) in any class wanting config data. The getConfig() accepts parameters so specific config data can be retrieved.

The class

NOTE: In code comments and later text, by "sub array" I mean one of the single sub arrays within the config array - such as the "php" array, or "router" array.

```
class coreConfig {

private $configData;
private $returnData;

private function setConfigData($key, $value = '') {

$configData = array (

'php' => array(
'current' => PHP_VERSION_ID,
'required' => '50400',
'test-

Introduction

First of all: well done you for writing your own framework!
(Just don't forget to throw it away once you feel you've learned enough). Configuration is, architecturally speaking, much more solid than relying on convention, so you score points there to!

Some minor issues

• Composer is a dependency management tool but it has some added functionality like a class autoloader and requirement enforcement for PHP versions and/or PHP modules. Especially if your framework will consist of separate libraries it might be useful to leverage the power of Composer to allow users to defined which parts they would/wouldn't want to use instead of having to do this through custom configuration. I say "might" because it will most likely depend on how complex your configuration is/will become and how well versed you (and/or users of your code) are in Composer usage.

As for the specific example of the framework requiring a certain PHP version, I would use a Composer and simply state a PHP version as a Platform package.

• Configuration should definitely go in a text file somewhere but you are correct that you will need a code wrapper for easy access. To avoid abuse and to keep things simple, using a format that can not contain logic (like XML, INI, JSON or YAML) is preferable over using code, even if it is "just" a PHP array.

The text values can be parsed and passed to a configuration object which is then given to the framework as configuration. As you observed in the comments, this would effectively replace the setConfigData method with a (preferably immutable) data object.

• I would also suggest you might want to have separate objects for different kinds of data instead of having everything cobbled together. Instead of splitting the array into single sub arrays I would opt for having separate specific config objects per responsibility (php/router/etc). These would not have to be separate classes but they could be if you feel this improves your design. Whether or not this is wise (as opposed to your suggestion in the comments of having one object with specific functions per responsibility) depends mostly on how the config is going to be used elsewhere in the framework.

• You may also want to take a look at "immutable objects" (basically a class with no setters, with data being injected through constructor parameters).

• The convention is for classnames to start with an uppercase letter.

• Using var_export instead of print_r yields a more "natural" output format. That is to say, a format that read more like natural PHP and thus requires less of a mind-switch when reading.

Comment in regard to the code

Looking through the provided code, a view points of concerns:

• The function setConfigData doesn't actually do any "setting". It just returns a value. Also, the $value parameter would be better of being called $subKey or something similar, as it currently gives the illusion that $value will be assigned to $key. The logic could also be more elegant but that's a matter for another review.

• The local variable $configData would be better of as a class property.

• The class property $returnData isn't needed, just use a local variable in the getConfig() function instead.

Answers to your questions

1. I find it hard to judge either "Sane" or "Decent". I think "Clean" is more important: single responsibility, clear separation of concerns, no side-effects, etc. Reading Clean Code by Robert C. Martin can be invaluable there.

1. If the set method actually set anything this would be less clean as it causes side-effects. Since it is really only another getter, there's no real problem.

1. Yes. This can become problematic. The best thing for it is simply to learn how to write unit tests (it honestly isn't all that difficult) and having tests with your code will teach you how to create code that is easy to test.

1. Yes, the array in setConfigData() is set again every time data is requested by getConfig. This is because it is being used from local scope and not from the class scope since it does not have a $this-> in front of the variable name. You don't need to make the array an object to resolve this, just load it from class scope using $this->configData.

1. OK or not is mostly a point of view. Most folks will probably think it is fine. In my personal taste the conditions could be cleaner/less muddled.

1. Yes, MAJOR issues. Configuration should live outside of the class. It can be injected into an object at the point where it is being instantiated. You really don't want for me to edit the framework code when I want an application I build on top of it to alter its behaviour!

1. Personally I would not put all of that functionality into one method. I'd create separate methods for separate scenarios to keep the class signature (or interface) simple and easy to understand.

In closing

If you feel inclined to look at this from other angles, you should probably also take a look at other data

StackExchange Code Review Q#96162, answer score: 4