Solr
  1. Solr
  2. SOLR-107

Iterable NamedList with java5 generics

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.2
    • Component/s: None
    • Labels:
      None

      Description

      Iterators and generics are nice!

      this patch adds both to NamedList.java

      1. IterableNamedList.patch
        4 kB
        Ryan McKinley
      2. IterableNamedList.patch
        4 kB
        Ryan McKinley

        Activity

        Hide
        Hoss Man added a comment -

        i only briefly skimmed the patch, but a couple quick questions came to mind...

        1) instead of creating a new NameValuePair<T> interface, couldn't named list just impliment Iterable<Map.Entry<String,T>> ?

        2) for this bit of code...

        @@ -183,7 +185,7 @@
        Iterator iter = eset.iterator();
        while (iter.hasNext())

        { Map.Entry entry = (Map.Entry)iter.next(); - add(entry.getKey().toString(), entry.getValue()); + add(entry.getKey().toString(), (T)entry.getValue()); }

        return args.size()>0;
        }

        ...that's in "addAll(Map)" right? ... if we're genericizing NamedList with respect to T, then shouldn't the method sig change to "addAll(Map<?,T>)" ... which would eliminate the need for the cast right?

        3) there's an "addAll(NamedList)" too isn't there? .. shouldn't that method change to "addAll(NamedList<T>)" as well?

        (I think all of those would still work in the current code base using the generics default of Object for unspecified templates)

        Show
        Hoss Man added a comment - i only briefly skimmed the patch, but a couple quick questions came to mind... 1) instead of creating a new NameValuePair<T> interface, couldn't named list just impliment Iterable<Map.Entry<String,T>> ? 2) for this bit of code... @@ -183,7 +185,7 @@ Iterator iter = eset.iterator(); while (iter.hasNext()) { Map.Entry entry = (Map.Entry)iter.next(); - add(entry.getKey().toString(), entry.getValue()); + add(entry.getKey().toString(), (T)entry.getValue()); } return args.size()>0; } ...that's in "addAll(Map)" right? ... if we're genericizing NamedList with respect to T, then shouldn't the method sig change to "addAll(Map<?,T>)" ... which would eliminate the need for the cast right? 3) there's an "addAll(NamedList)" too isn't there? .. shouldn't that method change to "addAll(NamedList<T>)" as well? (I think all of those would still work in the current code base using the generics default of Object for unspecified templates)
        Hide
        Ryan McKinley added a comment -

        updated patch for 1,2, and 3

        Show
        Ryan McKinley added a comment - updated patch for 1,2, and 3
        Hide
        Yonik Seeley added a comment -

        Looks good, I just committed this.
        Thanks again Ryan!

        ps: if patches start in the trunk, it's easier for someone to commit it.

        Show
        Yonik Seeley added a comment - Looks good, I just committed this. Thanks again Ryan! ps: if patches start in the trunk, it's easier for someone to commit it.
        Hide
        Hoss Man added a comment -

        This bug was modified as part of a bulk update using the criteria...

        • Marked ("Resolved" or "Closed") and "Fixed"
        • Had no "Fix Version" versions
        • Was listed in the CHANGES.txt for 1.2

        The Fix Version for all 39 issues found was set to 1.2, email notification
        was suppressed to prevent excessive email.

        For a list of all the issues modified, search jira comments for this
        (hopefully) unique string: 20080415hossman2

        Show
        Hoss Man added a comment - This bug was modified as part of a bulk update using the criteria... Marked ("Resolved" or "Closed") and "Fixed" Had no "Fix Version" versions Was listed in the CHANGES.txt for 1.2 The Fix Version for all 39 issues found was set to 1.2, email notification was suppressed to prevent excessive email. For a list of all the issues modified, search jira comments for this (hopefully) unique string: 20080415hossman2

          People

          • Assignee:
            Unassigned
            Reporter:
            Ryan McKinley
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development