Details
-
Improvement
-
Status: Resolved
-
Major
-
Resolution: Fixed
-
Lucene.Net 3.0.3
-
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; }