Lucene.Net
  1. Lucene.Net
  2. LUCENENET-515

bring back TokenStream.GetAttribute(Type).

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Lucene.Net 3.0.3
    • Fix Version/s: Lucene.Net 3.6
    • Component/s: Lucene.Net Core
    • Labels:
      None

      Description

      I Have noticed that TokenStream.GetAttribute(Type) is gone in favor for TokenStream.GetAttribute<Type>();

      Obviously TokenStream.GetAttribute<Type>() is a better syntax; but it should not replace TokenStream.GetAttribute(Type), but instead compliment it... But this is a common pitfall, especially for non-.NET developers and people who have not run into the implications of this..

      In the case of Lucene.NET, it is properly unlikely that this will affect anyone, however, I consider what has been done bad practice... now why is that?

      Many would think like this:

      Before I had to do this old thing...

      TokenStream.GetAttribute(typeof(TermAttribute));

      And god I hated that, so now I replaced it with the much more beautiful:

      TokenStream.GetAttribute<TermAttribute>();

      And deleted that old hag of a method taking a Type... And now I am happy...

      BUT!...

      What when...

      Type myNotDefinedHereType = GetTypeFromSomeWhere();
      TokenSteam.GetAttribute.... ?????!?!?!?... Uhm... What now???? 
      

      Now you have to write a whole lot of reflection mess, use a dynamically compiled delegate using IL-Emit or the Mono Compiler as a Service...
      All of those 3 workarounds are generally just ugly...

      If you keept both...

      Type myNotDefinedHereType = GetTypeFromSomeWhere();
      TokenSteam.GetAttribute(myNotDefinedHereType);
      

      While it might be unlikely that it will ever be used, there is always the off case, and API's should support both...

      So instead of:

      public virtual T GetAttribute<T>() where T : IAttribute
          {
            Type key = typeof (T);
            if (!this.attributes.ContainsKey(key))
              throw new ArgumentException("This AttributeSource does not have the attribute '" + key.FullName + "'.");
            else
              return (T) this.attributes[key].Value;
          }
      

      Do:

      public T GetAttribute<T>() where T : IAttribute
      {
         return (T) GetAttribute(typeof(T));
      } 
      public virtual IAttribute GetAttribute(Type key)
          {
            //Note: Since Type is required to implement IAttribute, I would properly check that as well to be able to provide a more meaningfull error... However to speed things up, I would do it inside the if bellow...
            if (!this.attributes.ContainsKey(key))
              throw new ArgumentException("This AttributeSource does not have the attribute '" + key.FullName + "'.");
            else
              return (T) this.attributes[key].Value;
          }
      

        Activity

        No work has yet been logged on this issue.

          People

          • Assignee:
            Unassigned
            Reporter:
            Jens Melgaard
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development