Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm new to OOP PHP and I created a simple input helper class. And was wondering if it's good? Are my dependencies good?

Also the reason I'm not throwing an exception with the get method is that I will mainly use this class for form validation purposes.

I'll take any suggestion for improvement!

class Input {
    private $_get,
            $_post;

    public function __construct(Array $get, Array $post) {
        $this->_get  = $get;
        $this->_post = $post;
    }

    public function exists($type = 'POST') {
        switch($type) {
            case 'POST':
                return (!empty($this->_post)) ? true : false;
            break;
            case 'GET':
                return (!empty($this->_get)) ? true : false;
            break;
            default:
                return false;
            break;
        }
    }

    public function get($input, $source = 'POST') {
        if($source == 'POST' && isset($this->_post[$input])) {
            return $this->_post[$input];
        } elseif($source == 'GET' && isset($this->_get[$input])) {
            return $this->_get[$input];
        }
        return false;
    }
}

I figured I could made my code shorter:

class Input {
    private $_source;

    public function __construct(Array $source) {
        $this->_source = $source;
    }

    public function exists() {
        return !empty($this->_source);
    }

    public function get($input) {
        return (isset($this->_source[$input])) ? $this->_source[$input] : false;
    }
}
share|improve this question
add comment

3 Answers 3

up vote 4 down vote accepted

In Java and C# you can simply put

return !empty($this->_source);

and

return isset($this->_source[$input]);

since the evaluation of that is a boolean by itself and thus there is no need for the additional ternary operation. I would suspect that this is the same in PHP.

share|improve this answer
    
Thanks for the tip. You also made me aware of something that wasn't suppose to be. As far as the rest of my code, I think it's clean now. –  Kid Diamond Apr 25 at 17:42
    
No problem, pleasure to help. Keep in mind though that you shouldn't edit your question to reflect the new situation: it confuses future readers. If you have a new question you can always make a new post. –  Jeroen Vannevel Apr 25 at 17:46
    
Yea, I thought it wouldn't be worth to create an entire new question. But will keep that in mind the second time. –  Kid Diamond Apr 25 at 17:47
add comment

In this switch :

    switch($type) {
        case 'POST':
            return (!empty($this->_post)) ? true : false;
        break;
        case 'GET':
            return (!empty($this->_get)) ? true : false;
        break;
        default:
            return false;
        break;
    }

You should not have break after return. It cannot be executed and is thus dead code.

As Jeroen mentioned, ternaries to determine a boolean do not make sense, just return the boolean expression immediately.

Also, I see you realized in the shorter version that for one request you can only have GET or POST, so it does not make sense to pass both.

class Input {
    private $_source;

    public function __construct(Array $source) {
        $this->_source  = $source;
    }

    public function exists() {
        return !empty($this->_source)
    }

    public function get($input) {
        return isset($this->_source[$input])
    }
}

I am not sure how you can assign to _source if the variable name is $_source though.

share|improve this answer
add comment

Your input object is basically just a wrapper around an array, so why don't you just use one. There are many advantages to do so when you can, simplicity and predictability mainly, also you will just have access to all the other methods arrays have.

// Construction
$input = array(...)

// Checking if it exists
!empty($input)

// Getting an item from the input
$input['key']
share|improve this answer
add comment

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.