1
\$\begingroup\$

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;
    }
}
\$\endgroup\$
4
\$\begingroup\$

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.

\$\endgroup\$
3
  • \$\begingroup\$ 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. \$\endgroup\$ – Kid Diamond Apr 25 '14 at 17:42
  • \$\begingroup\$ 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. \$\endgroup\$ – Jeroen Vannevel Apr 25 '14 at 17:46
  • \$\begingroup\$ Yea, I thought it wouldn't be worth to create an entire new question. But will keep that in mind the second time. \$\endgroup\$ – Kid Diamond Apr 25 '14 at 17:47
2
\$\begingroup\$

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.

\$\endgroup\$
0
\$\begingroup\$

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']
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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