Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Can I do something better with this code? It's just a snippet of a library I wrote.

<?php
/*
 * Variable.class.php
 * (c) 2013 sinan eker`enter code here`
 * */
 class Variable extends Config {
     protected static $vars = array();
     final public static function variables(){
         return static::$vars;
     }
     final public static function clear(String $name){
         unset(static::$vars[$name->getString()]);
     }
     final public static function destruct(){
         unset(static::$vars);
         static::$vars = array();
     }
     final public static function override(String $name, $value){
         static::clear($name);
         static::register($name, $value);
     }
     final public static function get(String $name){
         $name = $name->getString();
         return (isset(static::$vars[$name]) ? static::$vars[$name] : null);
     }
     final public static function register(String $name, $value){
         return (!is_null(static::get($name)) ? false : static::$vars[$name->getString()] = $value);
     }
 }
share|improve this question

1 Answer 1

First of all, method and variable docblocks are recommended. Provide a description of each method, its parameters and their return values.

I would give your getter function Variable::variables() an active name like getVariables() and rename your Variable::get() method to getVariable(). Consistency is important and you want to avoid confusion with the magic getter __get().

Are you intentionally using the non-magic method Variable::destruct instead of the PHP magic method __destruct()?

This is more of a syntax preference, but I would use array_key_exists() instead of isset() when referring to the property static::$vars since I find the verbage more aligned with what you are testing.

The docblocks are the only important thing. Good work.

share|improve this answer
    
First of all, thank you! I would not use array_key_exists, isset is faster! –  sinaneker May 13 '13 at 21:03
    
also this is a class, for using without an object instance. so the static is entitled! –  sinaneker May 13 '13 at 21:11

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.