Class to manage config data to allow DI

Question

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-index' => 'php test data',
      ),

      'framework' => array(
        'name' => 'frameworky',
        'version' => '1.0',
        'date' => '01/01/2015',
        'email' => '[email protected]',
      ),

      'router' => array(
        'allowedchars' => 'A-z',
        'url-prefix' => 'http://',
        'test-index' => 'router test data',
      ),

    );


    if (!isset($configData[$key]) 
      || (!empty($value) && !isset($configData[$key][$value]))) {      
      return false;

    }elseif (empty($value)) {
      return $configData[$key];

    } else {
      return $configData[$key][$value];
    }

  }


  public function getConfig($request = array()) {

    $this->returnData = '';

    // No data requested
    if (empty($request)) {
      $this->returnData = false;

    } elseif (!is_array($request)) {

        // Returns 1 single sub array
        $this->returnData[$request] = $this->setConfigData($request);

    } else {


      // Array(s) requested
      foreach ($request as $key => $value) {

        if (!is_array($value)) {
          // Returns entire sub array
          $this->returnData[$value] = $this->setConfigData($value);

        } else {

          // Specific sub array value(s) requested
          foreach ($value as $subKey => $subValue) {

            // 3rd level sub arrays not expected
            if (is_array($subValue)) {
              $this->returnData[$subKey] = false;

            } else {

              // Returns sub array value
              $this->returnData[$key][$subValue] = $this->setConfigData($key, $subValue);

            }

          }

        }

      }


    }

    return $this->returnData;

  }


}

Options

Note: By "key" I mean send in a key which exists in the config array.

The options to send to the getConfig() method are:

1. A single key: Returns an entire single sub array
2. An array of single keys: Returns multiple entire single sub arrays
3. An array with a key and value(s), or multiple arrays with a key and value(s): Returns specific value(s) from the sub array(s) for each specified key
4. Any mixture of 2 and 3 can be sent in one call to the method

Here are some example usages based on the above options. The results are from print_r():

$obj = new coreConfig;

//*Option 1*:  
$config1 = $obj->getConfig('php');
// Returns single entire sub array:
Array (
  [php] => Array (
    [current] => 50609 [required] => 50400 [test-index] => php test data 
  )
)

//*Option 2*:  
$config2 = $obj->getConfig(array('php', 'router'));
// Returns 2 entire sub arrays:
Array (
  [php] => Array (
    [current] => 50609 [required] => 50400 [test-index] => php test data 
  )
    [router] => Array (
      [allowedchars] => A-z [url-prefix] => http:// 
      [test-index] => router test data 
    )
  )

//*Option 3*:  
$config3 = $obj->getConfig(array('php' => array('current', 'test-index')));
// Returns partial sub array:
Array ( 
  [php] => Array (
    [current] => 50609 [test-index] => php test data
    )
  ) 

//*Option 4*:  
$config4 = $obj->getConfig(array(
    'php' => array('current', 'test-index'), 
    'router'
  )
);
// Returns partial sub array, and entire sub array:
Array ( 
  [php] => Array ( 
    [current] => 50609 [test-index] => php test data
  )
  [router] => Array (
    [allowedchars] => A-z [url-prefix] => http:// 
    [test-index] => router test data
    )
  )

My Questions

1. Is my general approach sane and decent for retrieving system config data within a framework?
2. Is it acceptable using the public (get) method to call a private (set) method (same class) to set the config array and retrieve data?
   e.g. $this->returnData[$value] = $this->setConfigData($value);
3. Unit testing - I have no experience, but know of its existence and some coding practices to following to cater for unit testing. With my class, the config data cannot be mocked in a test, is that a problem for unit testing?
4. Does the code set the array in setConfigData() every time data is requested by getConfig()? If so, would this be resolved/improved if I made the array an object - i.e. $this-configData? In trying to potentially re-use objects
5. Is the setConfigData() method ok having those if/else? It's like that because it allows the setConfigData() method to return only the required data, whereas this functionality being in the getConfig() method meant getting the entire array to check if keys existed etc.
6. My main niggle is to add config settings for the framework, one has to edit an array within a class method. This seems.. fiddly for config, but there are also benefits of it being in a class (DI, DIC, etc). Do you see this as a major issue as a developer adding config data in a class method array?

7. Is it worthwhile having the class take in so many options? Such as full config array request, partial, and a mixture, etc, all in one method call.

   Or would it be better to instead have the class only accept a single value and return an entire single sub array for each method call, and then call the method as many times as needed?

   //*Currently*:  
   $allConfigData = $obj->getConfig('everything-I-could-possibly-need')
   
   //*Alternative with class only accepting and returning one full sub array*:  
   $configData1 = $obj->getConfig('data-I-need');
   $configData2 = $obj->getConfig('more-data-I-need');
   $configData3 = $obj->getConfig('additional-data');
   

   I know this is a bit broad, but I'm asking you as a seasoned developer if this could potentially be useful. Currently I do not need more than one config sub array in any class, but my framework is expanding so potentially this could become useful.

Answer 1

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.

2. 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.

3. 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.

4. 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.

5. 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.

6. 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!

7. 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 structures that can/could be used for configuration other than an array. For instance, the ARC config component uses a tree structure.

If any of my comments seem unclear or need more elaboration, don't hesitate to ask. I will update my answer as needed.

Furthermore, as you stated that building a framework was mostly about the educational values, I think you will agree that something as essential as configuration has a lot of lessons in it. You seem to be learning them quite well.

Keep it up!