You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by "Henning Schmiedehausen (JIRA)" <ji...@apache.org> on 2008/05/21 18:37:56 UTC

[jira] Created: (SHINDIG-294) GadgetFeatureRegistry needs not to be restricted on Sets

GadgetFeatureRegistry needs not to be restricted on Sets
--------------------------------------------------------

                 Key: SHINDIG-294
                 URL: https://issues.apache.org/jira/browse/SHINDIG-294
             Project: Shindig
          Issue Type: Improvement
          Components: Common Components (Java)
            Reporter: Henning Schmiedehausen
         Attachments: collection.patch

Actually, Collection is fine and it allows to pass in things like Arrays.asList(<stuff>).



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


[jira] Updated: (SHINDIG-294) GadgetFeatureRegistry needs not to be restricted on Sets

Posted by "Henning Schmiedehausen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-294?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henning Schmiedehausen updated SHINDIG-294:
-------------------------------------------

    Attachment: shindig-294-2.patch

updated patch for this issue. Per Santiago's advice, I've rebased my git-svn repository and this patch *should* apply with -p1. Please let me know if not.

> GadgetFeatureRegistry needs not to be restricted on Sets
> --------------------------------------------------------
>
>                 Key: SHINDIG-294
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-294
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Common Components (Java)
>            Reporter: Henning Schmiedehausen
>         Attachments: collection.patch, shindig-294-2.patch
>
>
> Actually, Collection is fine and it allows to pass in things like Arrays.asList(<stuff>).

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


[jira] Commented: (SHINDIG-294) GadgetFeatureRegistry needs not to be restricted on Sets

Posted by "Kevin Brown (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12598776#action_12598776 ] 

Kevin Brown commented on SHINDIG-294:
-------------------------------------

"resultsFound" does need to be a Set because it is used to flatten the dependency graph. If your needed is:

{"rpc", "opensocial-0.7"}

And your input is a List, you'd get back:

{"rpc", "core", "opensocial-0.7", "core", "opensocial-reference", "core", "rpc", "core"}

Resulting in rpc and core code being seen multiple times in the output.

Whereas if it was a set you'd get back:

{"core", "rpc", "opensocial-reference", "opensocial-0.7"}

A Collection is fine for "needed" and "missing" though, of course.

Also, we intentionally don't use final parameters (only final classes and members).

> GadgetFeatureRegistry needs not to be restricted on Sets
> --------------------------------------------------------
>
>                 Key: SHINDIG-294
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-294
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Common Components (Java)
>            Reporter: Henning Schmiedehausen
>         Attachments: collection.patch
>
>
> Actually, Collection is fine and it allows to pass in things like Arrays.asList(<stuff>).

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


[jira] Commented: (SHINDIG-294) GadgetFeatureRegistry needs not to be restricted on Sets

Posted by "Henning Schmiedehausen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12598975#action_12598975 ] 

Henning Schmiedehausen commented on SHINDIG-294:
------------------------------------------------

Actually, after getting some sleep and thinking about your first comment (https://issues.apache.org/jira/browse/SHINDIG-294?focusedCommentId=12598776#action_12598776), I tend to disagree with myself. Allowing Collections to be passed in for all parameters can be a good thing because it places policy in the hand of the caller. If you can only pass in a set, you are e.g. not able to find out if a library might be used multiple times. 

So I now believe, that having all three parameters as collection is a good thing, because it does not break anything (only if you insist on passing in a list, you are in trouble) but allows more flexibility. 

I attach a second patch with clarified java docs and the final qualifiers removed. 

> GadgetFeatureRegistry needs not to be restricted on Sets
> --------------------------------------------------------
>
>                 Key: SHINDIG-294
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-294
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Common Components (Java)
>            Reporter: Henning Schmiedehausen
>         Attachments: collection.patch
>
>
> Actually, Collection is fine and it allows to pass in things like Arrays.asList(<stuff>).

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


[jira] Commented: (SHINDIG-294) GadgetFeatureRegistry needs not to be restricted on Sets

Posted by "Kevin Brown (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12598883#action_12598883 ] 

Kevin Brown commented on SHINDIG-294:
-------------------------------------

The simplest reason is that it's not consistent with the existing code. 

The reason it's not used in the existing code is because most of the original code came from coding styles that the majority of us were previously comfortable with.

The reason those coding styles don't use it is because it doesn't have a lot of value. When final parameters were added (Java 1.1 I think?), it was mostly billed as an optimization opportunity and to make sure that the code didn't modify the variable. Today javac is sufficiently advanced to not need it.

That leaves it with the "non-modifying" (potentially more correct case). Unlike const in C++, final doesn't actually prevent you from modifying the input value, so there's no gain there. All it does is prevent you from doing this:

public void foo(final Bar bar) {
bar.doThing();
bar = new Bar();
bar.doThing();
}

...which seems to be of highly limited utility for the added burden of writing more code, does it not?

If everyone that's been writing code changes their mind and the majority of committers decide that using final everywhere is the way to go, we can go ahead and do that, but I don't think it gets us anything, and not using it is more consistent with the existing code.

> GadgetFeatureRegistry needs not to be restricted on Sets
> --------------------------------------------------------
>
>                 Key: SHINDIG-294
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-294
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Common Components (Java)
>            Reporter: Henning Schmiedehausen
>         Attachments: collection.patch
>
>
> Actually, Collection is fine and it allows to pass in things like Arrays.asList(<stuff>).

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


[jira] Updated: (SHINDIG-294) GadgetFeatureRegistry needs not to be restricted on Sets

Posted by "Henning Schmiedehausen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-294?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henning Schmiedehausen updated SHINDIG-294:
-------------------------------------------

    Attachment: collection.patch

> GadgetFeatureRegistry needs not to be restricted on Sets
> --------------------------------------------------------
>
>                 Key: SHINDIG-294
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-294
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Common Components (Java)
>            Reporter: Henning Schmiedehausen
>         Attachments: collection.patch
>
>
> Actually, Collection is fine and it allows to pass in things like Arrays.asList(<stuff>).

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


[jira] Closed: (SHINDIG-294) GadgetFeatureRegistry needs not to be restricted on Sets

Posted by "Kevin Brown (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SHINDIG-294?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Kevin Brown closed SHINDIG-294.
-------------------------------

    Resolution: Fixed

This has been addressed.

> GadgetFeatureRegistry needs not to be restricted on Sets
> --------------------------------------------------------
>
>                 Key: SHINDIG-294
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-294
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Common Components (Java)
>            Reporter: Henning Schmiedehausen
>         Attachments: collection.patch, shindig-294-2.patch
>
>
> Actually, Collection is fine and it allows to pass in things like Arrays.asList(<stuff>).

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


[jira] Commented: (SHINDIG-294) GadgetFeatureRegistry needs not to be restricted on Sets

Posted by "Henning Schmiedehausen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SHINDIG-294?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12598853#action_12598853 ] 

Henning Schmiedehausen commented on SHINDIG-294:
------------------------------------------------

you are obviously right. Will fix. 

Why no final parameters? Any particular reason (I got this habit from "Elements of Java Style" where it is listed as a perferred pattern).



> GadgetFeatureRegistry needs not to be restricted on Sets
> --------------------------------------------------------
>
>                 Key: SHINDIG-294
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-294
>             Project: Shindig
>          Issue Type: Improvement
>          Components: Common Components (Java)
>            Reporter: Henning Schmiedehausen
>         Attachments: collection.patch
>
>
> Actually, Collection is fine and it allows to pass in things like Arrays.asList(<stuff>).

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