lucenenet-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jens Melgaard (JIRA)" <j...@apache.org>
Subject [jira] [Comment Edited] (LUCENENET-515) bring back TokenStream.GetAttribute(Type).
Date Mon, 10 Dec 2012 09:51:22 GMT

    [ https://issues.apache.org/jira/browse/LUCENENET-515?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13527807#comment-13527807
] 

Jens Melgaard edited comment on LUCENENET-515 at 12/10/12 9:50 AM:
-------------------------------------------------------------------

[~ccurrens]

you can compact many things into "one liners"... that doesn't make them less of a mess...
The reason I say "A whole lot of" is A: Because of the steps required, B: Because it would
be unnecessary and only ends up adding overhead...

But to take the steps:

# First you have to get the type for IAttribute
# Then you have to verify that the type is assignable to that
## If not, then you need to break out (one way or another, throwing an exception makes sense)
# Then you have to get the type you wan't to call on.
# Then you have to get the method you wan't to call on that type
# Then you need to make a generic version of that method
# Then you need to call the method
# Then you need to return the value

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

I Realize however that this might be because the code is converted over form Java, one could
question that ain't that same method missing in JAVA?... Now I haven't coded Java since just
before 1.5 so I don't have a clue on what you would do in JAVA today, if there are other means
of calling a method like this where you only know the exact type at runtime... So I won't
question that...

In any case, if we just "hoped" the team on Lucene.NET realized this by them self, it might
never have happened, instead... why not report it as an improvement/defect or whatever so
they can improve on their API... Isn't that in the end a help?...

It is OK if it gets low priority to me...
                
      was (Author: jmelgaard):
    [~ccurrens]

you can compact many things into "one liners"... that doesn't make them less of a mess...
The reason I say "A whole lot of" is A: Because of the steps required, B: Because it would
be unnecessary and only ends up adding overhead...

But to take the steps:

# First you have to get the type for IAttribute
# Then you have to verify that the type is assignable to that
## If not, then you need to break out (one way or another, throwing an exception makes sense)
# Then you have to get the type you wan't to call on.
# Then you have to get the method you wan't to call on that type
# Then you need to make a generic version of that method
# Then you need to call the method
# Then you need to return the value

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

I Realize however that this might be because the code is converted over form Java, one could
question that ain't that same method missing in JAVA?... Now I haven't coded Java since just
before 1.5 so I don't have a clue on what you would do in JAVA today, if there are other means
of calling a method like this where you only know the exact type at runtime...

In any case, if we just "hoped" the team on Lucene.NET realized this by them self, it might
never have happened, instead... why not report it as an improvement/defect or whatever so
they can improve on their API... Isn't that in the end a help?...

It is OK if it gets low priority to me...
                  
> 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

Mime
View raw message