[
https://issues.apache.org/jira/browse/LUCENENET-515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13528169#comment-13528169
]
Christopher Currens edited comment on LUCENENET-515 at 12/10/12 7:09 PM:
-------------------------------------------------------------------------
Jens, you've missed my point. We do need to add this, and I was just trying to
be helpful by suggesting a simple solution, but instead I still am met with
resistance from you, _even though I agreed with you about adding an overload
that accepted a type as a parameter._ I suggested this simple workaround
because I have my doubts that this would be added into a 3.0.3 maintenance
release because it's such a low priority and we have little resources to spend
on these things when we're focused on trying to port 3.6 and/or 4.0.
{quote}
And yes, you did all in a very few lines of code, it doesn't change that you DO
all that... as opposed to:
# Call the method
# Return the value
{quote}
As opposed to just calling the method and returning the value? So where do you
check if the type passed into the method is assignable from {{IAttribute}}?
You're oversimplifying things here, because it still has to be done somewhere.
However, there's no doubt that reflection would be slower than having a method
dedicated to this with all of the proper checks and casting, but on the flip
side, you can use {{Delegate.CreateDelegate}} and cache the result. Then, the
only real additional cost is reflection required to initially create it.
Subsequent runs would be as fast as a normal call (at least in this case. If
the type's method body were shorter, it would have the opportunity to be
inlined where the delegate would not).
Anyway, this is slowly moving to a pointless argument about reflection, so,
here's the bottom line:
*We will add an additional non-generic method overload for these generic ones.*
This could be done in a few short months, a year, or perhaps even longer. I
would prefer months, but sometimes it just can't happen. In the meantime if
you decide that you can't live without this, you can _easily_ write code that
uses reflection to get this done (it took me all of 10 minutes to write the
above code). Any reflection code you write can be viewed as temporary, since
*we are going to add this additional non-generic method overload* (I feel like
I can't stress this enough). If you implement the reflection correctly, you
will likely see no real measurable gain in performance when you are able to
switch to using that non-generic method overload in the future.
was (Author: ccurrens):
Jens, you've missed my point. I was trying to be helpful by suggesting a
simple solution, but instead I still am met with resistance from you, _even
though I agreed with you about adding an overload that accepted a type as a
parameter._ I suggested this simple workaround because I have my doubts that
this would be added into a 3.0.3 maintenance release because it's such a low
priority and we have little resources to spend on these things when we're
focused on trying to port 3.6 and/or 4.0.
{quote}
And yes, you did all in a very few lines of code, it doesn't change that you DO
all that... as opposed to:
# Call the method
# Return the value
{quote}
As opposed to just calling the method and returning the value? So where do you
check if the type passed into the method is assignable from {{IAttribute}}?
You're oversimplifying things here, because it still has to be done somewhere.
However, there's no doubt that reflection would be slower than having a method
dedicated to this with all of the proper checks and casting, but on the flip
side, you can use {{MethodInfo.CreateDelegate}} and cache the result. Then,
the only real additional cost is reflection required to initially create it.
Subsequent runs would be as fast as a normal call (at least in this case. If
the type's method body were shorter, it would have the opportunity to be
inlined where the delegate would not).
Anyway, this is slowly moving to a pointless argument about reflection, so,
here's the bottom line:
*We will add an additional non-generic method overload for these generic ones.*
This could be done in a few short months, a year, or perhaps even longer. In
the meantime if you decide that you can't live without this, you can _easily_
write code that uses reflection to get this done (it took me all of 10 minutes
to write the above code). Any reflection code you write can be viewed as
temporary, since *we are going to add this additional non-generic method
overload* (I feel like I can't stress this enough). If you implement the
reflection correctly, you will likely see no real measurable gain in
performance when you are able to switch to using that non-generic method
overload in the future.
> 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
>
> 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
