Re: [php-49] Does this look ok for my constructor, or is it too hard to read?

From: Tim Piele
Sent on: Thursday, February 14, 2013 9:02 AM
Does this constructor make my IndexAction look fat?

I work in CodeIgniter mainly (Zend at work but we won't talk about that)... 

I use my constructors to load helpers and libraries needed down below and set object variables like $this->member_id or whatever... 

I put validation code down below and then call it explicitly when needed.




On Thu, Feb 14, 2013 at 8:47 AM, Dan Munro <[address removed]> wrote:
I think your code looks good JD as far as being readable and logical. I'm curious, what is the purpose of the class? Will you need that xml/json formatting validation logic elsewhere too? I'm just wondering if putting the validation code directly in constructor is the best location for it or the best time. I'd consider, what is the purpose of the constructor? I know a lot of people say make them as minimal as possible, which I agree with most of the time. Would it make more sense to have a class that just validates the formatting of a datasource? In that case, maybe it makes sense to have a private class property like "isDataValidated" in your current class (default false), and whenever the class actually tries to use the data, then validate it and flip the switch to true (and check that switch so that you don't validate the data more than once).

Anyway those are just some thoughts. I think your code is good as it is but if you want to consider some alternative ways to write/handle responsibilities then those ideas are probably a good start.

Just my 2 shillings


On Thu, Feb 14, 2013 at 8:02 AM, Jd Daniel <[address removed]> wrote:

Thanks Mike :)

On Feb 14, 2013 7:59 AM, "Mike Blaszczak" <[address removed]> wrote:
strpos() returns an int, not a bool. Seems a bit inconsistent to test NULL explicitly but not test the return from strpos() for != 0 explicitly.

Comments like "// !!!" could be more helpful.


On Wed, Feb 13, 2013 at 10:46 AM, Jd Daniel <[address removed]> wrote:
I know I overcomplicate stuff sometimes, just wanna make sure i'm not off the deep end for other devs. Cheers.

    /**
     * Creates a new Private_Service_Foo_Response object.
     *
     * @param string $data Response data returned by Foo
     * @param array  $info HTTP {@link http://us3.php.net/curl_getinfo metadata}
     *
     * @throws Private_Service_Foo_Exception
     */
    public function __construct($data, array $info)
    {
        $this->_dataString = $data;
        $this->_httpInfo   = $info;

        // If no error occurred, attempt to convert returned data to an object
        if(!$this->isError() && isset($info['url']))
        {
            // Convert JSON to StdClass object, XML to SimpleXMLElement
            if(strpos($info['url'], '.json'))
            {
                // throw error on bad json
                if (NULL !== ($this->dataObject = json_decode($data)) && JSON_ERROR_NONE === json_last_error())
                {
                    self::_throwException('Error parsing JSON file ' . $file . ': ' . json_last_error());
                }
            }
            else if(strpos($info['url'], '.xml'))
            {
                // throw error on bad xml
                libxml_use_internal_errors(true); // !!!
                if (FALSE === ($this->dataObject = simplexml_load_string($data)))
                {
                    self::_throwException('Error parsing XML file ' . $file . ': ' . implode(', ', libxml_get_errors()));
                }
            }
        }
    }


--
Jd Daniel || ERADO
Senior Applications Architect
7901 Delridge Way SW #36D, Seattle
C.    [masked]





--
Please Note: If you hit "REPLY", your message will be sent to everyone on this mailing list ([address removed])
This message was sent by Jd Daniel ([address removed]) from The Seattle PHP Meetup Group.
To learn more about Jd Daniel, visit his/her member profile
Set my mailing list to email me As they are sent | In one daily email | Don't send me mailing list messages

Meetup, POB 4668 #37895 NY NY USA 10163 | [address removed]





--
Please Note: If you hit "REPLY", your message will be sent to everyone on this mailing list ([address removed])
This message was sent by Mike Blaszczak ([address removed]) from The Seattle PHP Meetup Group.
To learn more about Mike Blaszczak, visit his/her member profile
Set my mailing list to email me As they are sent | In one daily email | Don't send me mailing list messages

Meetup, POB 4668 #37895 NY NY USA 10163 | [address removed]




--
Please Note: If you hit "REPLY", your message will be sent to everyone on this mailing list ([address removed])
This message was sent by Jd Daniel ([address removed]) from The Seattle PHP Meetup Group.
To learn more about Jd Daniel, visit his/her member profile
Set my mailing list to email me As they are sent | In one daily email | Don't send me mailing list messages

Meetup, POB 4668 #37895 NY NY USA 10163 | [address removed]



--
From the desk of Dan Munro




--
Please Note: If you hit "REPLY", your message will be sent to everyone on this mailing list ([address removed])
This message was sent by Dan Munro ([address removed]) from The Seattle PHP Meetup Group.
To learn more about Dan Munro, visit his/her member profile

Our Sponsors

People in this
Meetup are also in:

Log in

Not registered with us yet?

Sign up

Meetup members, Log in

By clicking "Sign up" or "Sign up using Facebook", you confirm that you accept our Terms of Service & Privacy Policy