Skip to content
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.

Sign up for GitHub

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

more changes to SAMTagUtil #1227

Merged
merged 4 commits into from
Nov 16, 2018
Merged

more changes to SAMTagUtil #1227

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
db83cff
more changes to SAMTagUtil
lbergelson Nov 14, 2018
3e20504
more stuff
lbergelson Nov 15, 2018
c2c19db
fix missed message
lbergelson Nov 15, 2018
468cd3b
Apply suggestions from code review
pshapiro4broad Nov 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/htsjdk/samtools/SAMBinaryTagAndUnsignedArrayValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public SAMBinaryTagAndUnsignedArrayValue(final short tag, final Object value) {
if (!value.getClass().isArray() || value instanceof float[]) {
throw new IllegalArgumentException("Attribute type " + value.getClass() +
" cannot be encoded as an unsigned array. Tag: " +
SAMTagUtil.makeStringTag(tag));
SAMTag.makeStringTag(tag));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/main/java/htsjdk/samtools/SAMBinaryTagAndValue.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ public SAMBinaryTagAndValue(final short tag, final Object value) {
}
if (!isAllowedAttributeValue(value)) {
throw new IllegalArgumentException("Attribute type " + value.getClass() + " not supported. Tag: " +
SAMTagUtil.makeStringTag(tag));
SAMTag.makeStringTag(tag));
}
this.tag = tag;
this.value = value;
Expand Down
18 changes: 9 additions & 9 deletions src/main/java/htsjdk/samtools/SAMRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1163,7 +1163,7 @@ public boolean hasAttribute(final String tag) {
* @return Appropriately typed tag value, or null if the requested tag is not present.
*/
public Object getAttribute(final String tag) {
return getAttribute(SAMTagUtil.makeBinaryTag(tag));
return getAttribute(SAMTag.makeBinaryTag(tag));
}

/**
Expand Down Expand Up @@ -1197,7 +1197,7 @@ public Integer getIntegerAttribute(final String tag) {
* @throws {@link htsjdk.samtools.SAMException} if the value is out of range for a 32-bit unsigned value, or not a Number
*/
public Long getUnsignedIntegerAttribute(final String tag) throws SAMException {
return getUnsignedIntegerAttribute(SAMTagUtil.makeBinaryTag(tag));
return getUnsignedIntegerAttribute(SAMTag.makeBinaryTag(tag));
}

/**
Expand All @@ -1220,11 +1220,11 @@ public Long getUnsignedIntegerAttribute(final short tag) throws SAMException {
return lValue;
} else {
throw new SAMException("Unsigned integer value of tag " +
SAMTagUtil.makeStringTag(tag) + " is out of bounds for a 32-bit unsigned integer: " + lValue);
SAMTag.makeStringTag(tag) + " is out of bounds for a 32-bit unsigned integer: " + lValue);
}
} else {
throw new SAMException("Unexpected attribute value data type " + value.getClass() + " for tag " +
SAMTagUtil.makeStringTag(tag));
SAMTag.makeStringTag(tag));
}
}

Expand Down Expand Up @@ -1375,7 +1375,7 @@ public float[] getFloatArrayAttribute(final String tag) {
* @throws SAMException if the tag is not present.
*/
public boolean isUnsignedArrayAttribute(final String tag) {
final SAMBinaryTagAndValue tmp = this.mAttributes.find(SAMTagUtil.makeBinaryTag(tag));
final SAMBinaryTagAndValue tmp = this.mAttributes.find(SAMTag.makeBinaryTag(tag));
if (tmp != null) return tmp.isUnsignedArray();
throw new SAMException("Tag " + tag + " is not present in this SAMRecord");
}
Expand Down Expand Up @@ -1420,7 +1420,7 @@ public Object getAttribute(final short tag) {
* String values are not validated to ensure that they conform to SAM spec.
*/
public void setAttribute(final String tag, final Object value) {
setAttribute(SAMTagUtil.makeBinaryTag(tag), value);
setAttribute(SAMTag.makeBinaryTag(tag), value);
}

/**
Expand All @@ -1436,7 +1436,7 @@ public void setUnsignedArrayAttribute(final String tag, final Object value) {
if (Array.getLength(value) == 0) {
throw new IllegalArgumentException("Empty array passed to setUnsignedArrayAttribute for tag " + tag);
}
setAttribute(SAMTagUtil.makeBinaryTag(tag), value, true);
setAttribute(SAMTag.makeBinaryTag(tag), value, true);
}

/**
Expand Down Expand Up @@ -1557,7 +1557,7 @@ public List<SAMTagAndValue> getAttributes() {
SAMBinaryTagAndValue binaryAttributes = getBinaryAttributes();
final List<SAMTagAndValue> ret = new ArrayList<>();
while (binaryAttributes != null) {
ret.add(new SAMTagAndValue(SAMTagUtil.makeStringTag(binaryAttributes.tag),
ret.add(new SAMTagAndValue(SAMTag.makeStringTag(binaryAttributes.tag),
binaryAttributes.value));
binaryAttributes = binaryAttributes.getNext();
}
Expand Down Expand Up @@ -1771,7 +1771,7 @@ private void addField(final StringBuilder buffer, final String field) {
}

private static String formatTagValue(final short tag, final Object value) {
final String tagString = SAMTagUtil.makeStringTag(tag);
final String tagString = SAMTag.makeStringTag(tag);
if (value == null || value instanceof String) {
return tagString + ":Z:" + value;
} else if (value instanceof Integer || value instanceof Long ||
Expand Down
30 changes: 27 additions & 3 deletions src/main/java/htsjdk/samtools/SAMTag.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
*/
package htsjdk.samtools;

import htsjdk.samtools.util.StringUtil;

/**
* The standard tags for a SAM record that are defined in the SAM spec.
*/
Expand Down Expand Up @@ -105,24 +107,46 @@ public enum SAMTag {
U2,
UQ;

private final short shortValue = SAMTag.makeBinaryTag(this.name());;
// Cache of already-converted tags. Should speed up SAM text generation.
// Not synchronized because race condition is not a problem.
private static final String[] stringTags = new String[Short.MAX_VALUE];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

static fields should be uppercase

Suggested change
private static final String[] stringTags = new String[Short.MAX_VALUE];
private static final String[] STRING_TAGS = new String[Short.MAX_VALUE];

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't really a constant though. My thought is that things that are constant are final, but this is a static cache that's updated with new values. It seems more confusing to make it caps since people assume those are immutable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I usually use all caps for any statics, since it's important to know what things are static, but I can see that all caps is typically used for constants only.


private final short shortValue = SAMTag.makeBinaryTag(this.name());

/**
* Convert from String representation of tag name to short representation.
*
* @param tag 2-character String representation of a tag name.
* @return Tag name packed as 2 ASCII bytes in a short.
*/
static short makeBinaryTag(String tag) {
public static short makeBinaryTag(String tag) {
if (tag.length() != 2) {
throw new IllegalArgumentException("String tag does not have length() == 2: " + tag);
}
return (short)(tag.charAt(1) << 8 | tag.charAt(0));
}

/**
* Convert from short representation of tag name to String representation.
*
* @param tag Tag name packed as 2 ASCII bytes in a short.
* @return 2-character String representation of a tag name.
*/
public static String makeStringTag(final short tag) {
String ret = stringTags[tag];
if (ret == null) {
final byte[] stringConversionBuf = new byte[2];
stringConversionBuf[0] = (byte)(tag & 0xff);
stringConversionBuf[1] = (byte)((tag >> 8) & 0xff);
ret = StringUtil.bytesToString(stringConversionBuf);
stringTags[tag] = ret;
}
return ret;
}

/**
* Get the binary representation of this tag name.
* @see SAMTagUtil#makeBinaryTag(String)
* @see SAMTag#makeBinaryTag(String)
*
* @return the binary representation of this tag name
*/
Expand Down
Loading