htsjdk
htsjdk copied to clipboard
Some improvements on CigarOperator enum.
Description
- Clean up a bit the code.
- Removed per value switches.
- String, character initialization.
- Optimize some operations
- Fast look-up of values from primitive codes.
- Static to member methods to generate primitive codes.
- Deprecated methods and update their uses to new ones.
- Clear most of the warnings on the main and test code.
Things to think about before submitting:
- [ ] Make sure your changes compile and new tests pass locally.
- [ ] Add new tests or update existing ones:
- A bug fix should include a test that previously would have failed and passes now.
- New features should come with new tests that exercise and validate the new functionality.
- [ ] Extended the README / documentation, if necessary
- [ ] Check your code style.
- [ ] Write a clear commit title and message
- The commit message should describe what changed and is targeted at htsjdk developers
- Breaking changes should be mentioned in the commit message.
Codecov Report
Merging #1448 into master will decrease coverage by
0.001%
. The diff coverage is86.667%
.
@@ Coverage Diff @@
## master #1448 +/- ##
===============================================
- Coverage 68.396% 68.394% -0.001%
+ Complexity 8497 8480 -17
===============================================
Files 583 583
Lines 34413 34418 +5
Branches 5729 5741 +12
===============================================
+ Hits 23537 23540 +3
Misses 8653 8653
- Partials 2223 2225 +2
Impacted Files | Coverage Δ | Complexity Δ | |
---|---|---|---|
src/main/java/htsjdk/samtools/util/CigarUtil.java | 30.303% <0%> (ø) |
14 <0> (ø) |
:arrow_down: |
...rc/main/java/htsjdk/samtools/BinaryCigarCodec.java | 95.652% <100%> (ø) |
8 <0> (ø) |
:arrow_down: |
src/main/java/htsjdk/samtools/TextCigarCodec.java | 92% <100%> (ø) |
11 <0> (ø) |
:arrow_down: |
src/main/java/htsjdk/samtools/CigarOperator.java | 90% <87.5%> (-6.923%) |
32 <26> (-18) |
|
src/main/java/htsjdk/samtools/BAMFileReader.java | 68.466% <0%> (+0.852%) |
52% <0%> (+1%) |
:arrow_up: |
I have addressed some the comments and thought of another way to address broader operator categories meaning alignment, indel, clips, etc... It turns out that Alignment, Indel, Clipping, Padding and Skip (N) are disjointed and exhaustive categorization of the operators.
I added a non-public enum for those categories and the boolean final fields for each status accessor (e.g. isAlignment, isIndel... etc) is initialized using a single param in the constructor that indicates the class/category for that operator.
What do you think? @lbergelson @yfarjoun ...
You know what they say ... if you feel you need to ask to do somethings is because you know you shouldn't do it :P