[
https://issues.apache.org/jira/browse/LUCENENET-515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13530170#comment-13530170
]
Simon Svensson commented on LUCENENET-515:
------------------------------------------
Why is it a nightmare to have two virtual methods? Is it an overloading
problem, or just an issue if someone does a faulty implementation by overriding
GetAttribute<T>() but not GetAttribute(Type)? I'll have to admit that I didn't
think about that, but I guess the clean solution (code-wise) would be removing
virtual from the generic version, since it just calls the non-generic version
anyway. This would break backwards-compatibility, but the changes would be
minor (personal bias!) for implementors.
Removing virtual from the non-generic version would mean that it needed some
reflection to call the generic version, so the behavior can be overriden.
The only way to avoid checking the attribute type twice would be to either 1)
duplicate the code, or 2) add an additional method that does the actual
fetching, which does not validation of the attribute type. The validation would
then be done by the entry method. Both these sounds ugly code-wise.
> bring back TokenStream.GetAttribute(Type).
> ------------------------------------------
>
> Key: LUCENENET-515
> URL: https://issues.apache.org/jira/browse/LUCENENET-515
> Project: Lucene.Net
> Issue Type: Improvement
> Components: Lucene.Net Core
> Affects Versions: Lucene.Net 3.0.3
> Reporter: Jens Melgaard
> Fix For: Lucene.Net 3.6
>
>
> 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...
> {code}TokenStream.GetAttribute(typeof(TermAttribute));{code}
> And god I hated that, so now I replaced it with the much more beautiful:
> {code}TokenStream.GetAttribute<TermAttribute>();{code}
> And deleted that old hag of a method taking a Type... And now I am happy...
> BUT!...
> What when...
> {code}
> Type myNotDefinedHereType = GetTypeFromSomeWhere();
> TokenSteam.GetAttribute.... ?????!?!?!?... Uhm... What now????
> {code}
> 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...
> {code}
> Type myNotDefinedHereType = GetTypeFromSomeWhere();
> TokenSteam.GetAttribute(myNotDefinedHereType);
> {code}
> 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:
> {code}
> 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;
> }
> {code}
> Do:
> {code}
> 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;
> }
> {code}
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
