-
Notifications
You must be signed in to change notification settings - Fork 244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
improve gff: add Locatable to Gff3BaseData, add 'Optional<String> getAttr(key)' and 'hasAttribute' #1726
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@lindenb This seems like a good idea but can we change the method name? getAttr and getAttribute are to confusing together. Something like getSingleAttribute()
might be clearer?
* @return <tt>Optional<String></tt> if this map contains zero or one attribute for the specified key | ||
* @throws IllegalArgumentException if there is more than one value | ||
*/ | ||
public Optional<String> getAttr(final String key) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea. Can we change the name though? Maybe getSingleAttribute()?
or getUniqueAttribute() |
@lbergelson thanks for the review ! I changed the name to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Thanks for all these additions @lindenb |
Description
This PR improves the GFF3 classes:
List<String> getAttribute(key)
can be cumbersome. This PR adds a new functionOptional<String> getAttr(key)
, it returns an Optional if there is zero or one value. Otherwise an exception is thrown.hasAttribute
to test for the presence of an attribute.implements Locatable
to Gff3BaseDataThings to think about before submitting: