Implementing ArrayAccess for Config

[ragboyjr]

This syntax allows for less verbose accessing of config items. The only issue I see is that offsetGet will return NULL if it doesn't exist whereas $this->config->item('item'); will return false.

[AkenRoberts]

Anything that checks for array offsets should use array_key_exists() instead of isset(), due to false negatives from NULL values.

Related: the config class' item() method has this problem, too, which should be fixed.

[narfbg]

CI's styleguide doesn't accept camelCase. And of what use is this anyway?

[TheDigitalOrchard]

@narfbg, when implementing ArrayAccess, those methods, which are written in camelCase in the documentation, need to be implemented.

I'm split on whether this should be included in CI. It sets a bad precedence for configuration for other areas of CI, such as individual libraries, etc. I think it should remain in a fork or overloaded Config library, but then again, there's no way to implement it in a class extension anyway.... hmmm.

[AkenRoberts]

Missing second parameter.

[AkenRoberts]

@TheDigitalOrchard Sure you can.

class MY_Config extends CI_Config implements ArrayAccess {}

[vlakoff]

By the way, shouldn't config->item('item') return NULL instead of FALSE if not set, as it has already been changed in load->get_var() ?

[ragboyjr]

Um, sorry for the period of silence, but I'm fine if this feature doesn't get implemented; however, it's nice syntactic sugar, doesn't affect runtime performance, doesn't break old code, and doesn't affect people who don't want to use that feature, and it's a pretty neat feature for people who want it. If that argument doesn't sound that great to everyone else, then I'm totally fine with closing the ticket.

[AkenRoberts]

If it is to be accepted, you still need to make a couple updates to make it ready to merge.

[narfbg]

I'm still having a hard time understanding this and the included documentation changes doesn't make it any clearer. If I can't understand the benefits, how could a newbie reading the docs do?

[ragboyjr]

This feature is primarily for syntactic sugar. We can update the documentation to be clearer, but it's just a simpler and more concise way of accessing the main config array. It doesn't affect runtime performance, and it's the same time complexity as using Config::item(). So the advantages would be simpler, and concise syntax for accessing config values. The disadvantage would be that it might confuse new comers to ci, however I think we can combat that last point by making documentation clearer, and if it still confuses a certain newcomer, they aren't forced to use this syntax, they could use item function.

[narfbg]

OK, consider me a complete newbie and answer to these questions:

• How is it different than Config::item()?
• If it does the same thing as Config::item(), why should I use it?
• Can I have an example?

[vlakoff]

FYI, I proposed for config->item() a change of the return value for nonexistent items: #2449.

[TheDigitalOrchard]

1. Config::item() should indeed be returning NULL if a config key doesn't exist. If this isn't yet happening, then it should be fixed.
2. Implementing ArrayAccess would be useful in cases where you want to concatenate a config-stored value into a string, but that's the only use that I can think of where Config::item() doesn't already meet the need.

$str = "This string is {$this->config['str_state']}";

vs:

$str = "This string is ".$this->config->item('str_state');

How often that feature would be used in this way is to be established.

[AkenRoberts]

Basically, there's no new value or feature added, it just gives users another option for how to access / define config items. It's possible that some people would prefer array notation; this would give them the option.

[ragboyjr]

@cryode and @TheDigitalOrchard is correct. It's pretty much syntactic sugar, it doesn't affect runtime performance, using it is just efficient as Config::item(), and IMHO, it's nice, concise way to access config items; I also think it makes sense to use this array notation because the Config class is really just a wrapper around the main config array (and sub arrays that are loaded).

[TheDigitalOrchard]

@ragboyjr - it appears you added a commit with totally unrelated changes from this pull request. Was that a mistake?

[ragboyjr]

@TheDigitalOrchard, I'm pretty sure that Merge was because I was just updating my CI repo to the latest in development