[ 
https://issues.apache.org/jira/browse/SHINDIG-355?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12609854#action_12609854
 ] 

Chris Chabot commented on SHINDIG-355:
--------------------------------------

OAuthStore.php, function equals (2 of them, same problems):
                if ($this == $obj)
                        return true;
                if ($obj == null)
                        return false;
                        /*?*/           if (getClass() != $obj->getClass())
                        return false;
                
please always use {}'s even in simple statements like that, because it leads to 
bugs in the long run when you dont, the whole function is littered with missing 
{ }'s, makes it hard to read and maintain!

                if ($gadgetUri == null) {
                        if ($other->gadgetUri != null)
                                return false;
                } else ... etc

same lacking $this-> scope ... i guess this function was never used and called? 
If so why is it in there? :)

Same comment about equals goes for hashCode():
        public function hashCode()
        {
                $prime = 31;
                $result = 1;
                $result = $prime * $result + (($gadgetUri == null) ? 0 : 
$gadgetUri->hashCode());
                $result = $prime * $result + (($serviceName == null) ? 0 : 
$serviceName->hashCode());
                return $result;
        }

Also forgot $this-> scope .. if your doing a straight port of java code ... 
always think 'do i need this code' (in equals and hashCode you often don't, PHP 
== compares objects just fine on properties, no need to build that your self), 
and 'whats the scope of this variable'. In Java you can often omit the 
this/self/etc, but in php you can't :)

class ProviderKey, function equals:
                if (getClass() != obj . getClass()) {
                        return false;
                }

that's not even valid php code! :-) 

In class TokenKey, function equals you do:
if (getClass() != $obj->getClass()) {
                        return false;
                }

which also isn't valid PHP code ... if you DID want to build such a thing and 
compare class names, use the get_class($object) function please.

I'm assuming (because of the non-valid PHP code and the fact it could never 
have worked) these functions are dead code that is never called... So i'll 
leave them out.

Please be more careful with that in the future and audit your code before 
submitting :)

> OAuth in gadget xml spec 
> -------------------------
>
>                 Key: SHINDIG-355
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-355
>             Project: Shindig
>          Issue Type: New Feature
>          Components: Gadget Rendering Server (PHP)
>            Reporter: Chris Chabot
>         Attachments: feature-SHINDIG-355.patch
>
>
> Instead of having a new oauth feature, the proposal is to put it directly 
> into the gadget xml spec. Should parse and support the oauth feature.
> See: 
> http://www.google.com/url?q=http://groups.google.com/group/opensocial-and-gadgets-spec/browse_frm/thread/a1ce63eecfb21cad&sa=D&usg=ALhdy2-tUxagajVfd0hx98L5CubK3S340Q

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Reply via email to