[
https://issues.apache.org/jira/browse/LUCENENET-515?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Christopher Currens reopened LUCENENET-515:
-------------------------------------------
[~jmelgaard] Ditto.
[~sisve] - Thanks for that. I'm glad you also made the change for
{{HasAttribute}} as well. I'm debating in my mind whether or not
{{AddAttribute}} needs one as well. There is {{AddAttributeImpl}} which takes
in an {{Attribute}} _class_, but all of these other methods take in an
interface. Perhaps it might be a good idea to add one of those as well? I can
see people using reflection to get only the interfaces and at that point, they
wouldn't have a method to use. If you agree that this is something we should
also implement, let me know before you start working on any code with it,
making that change has a lot of different implications on the code, and I'd
like to discuss it before we go in too deep.
Something I noticed in the code now, is that having dual virtual methods for
{{GetAttribute}} and {{HasAttribute}} is a nightmare. This is the real reason
I felt it was necessary to reopen the issue, since there are complications to
removing one as virtual. We have to consider what breaking changes this might
entail. Since 3.0.3 was released a while ago, marking the generic version as
non-virtual would break code that was just updated from the method we removed,
as part of the hefty API changes from 2.9.4 to 3.x. However, removing virtual
from {{GetAttribute(Type)}} would solve backwards compatibility, breaking
changes, and perhaps also porting headaches, but would require reflection in
order to DRY.
The internal classes in Lucene use the generic {{XXXAttribute}} methods, so we
should take care in making changes that will affect the speed of those. In the
case of {{GetAttribute<T>}} calling the non-generic version, that code winds up
making unneeded checks to the variable that the compiler is already doing for
free. The speed difference is quite negligible with these two extra checks so
this just an example and not an issue. We just need to keep that in mind while
we make changes to this class.
> 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
