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

Jump to bottom

Support Range Matching by Attributes.matches(Attributes keys,..) #125

Merged
merged 2 commits into from
Jun 30, 2017
Merged

Support Range Matching by Attributes.matches(Attributes keys,..) #125

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95bfcc5
Support Range Matching by Attributes.matches(Attributes keys,..) dcm4…
gunterze Jun 30, 2017
84b9726
Support Range Matching by Attributes.matches(Attributes keys,..) dcm4…
gunterze Jun 30, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 18 additions & 2 deletions dcm4che-core/src/main/java/org/dcm4che3/data/Attributes.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -790,7 +790,7 @@ public String[] getStrings(String privateCreator, int tag, VR vr) {

private static String[] toStrings(Object val) {
return (val instanceof String)
? new String[] { (String) val }
? new String[] { (String) val }
: (String[]) val;
}

Expand Down Expand Up @@ -2943,9 +2943,19 @@ private boolean matches(String privateCreator, int tag, VR vr,

boolean ignoreCase = ignorePNCase && vr == VR.PN;
for (String keyVal : keyVals) {
DateRange dateRange = null;
if (vr == VR.PN)
keyVal = new PersonName(keyVals[0]).toString();
Copy link
Member

Choose a reason for hiding this comment

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

I think this if is now handled in the switch below, so the if can be removed.


switch (vr) {
case PN:
keyVal = new PersonName(keyVals[0]).toString();
break;
case DA:
case DT:
case TM:
dateRange = toDateRange(keyVal, vr);
Copy link
Member

Choose a reason for hiding this comment

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

I'd also add a break here for safety reasons (should someone in the future add additional cases the break could be forgotten), and maybe even //$FALL-THROUGH$ comments to the DA and DT cases, to clearly mark the intention.
This is what both Oracle and Google recommend in their code guidelines:
https://google.github.io/styleguide/javaguide.html#s4.8.4-switch
http://www.oracle.com/technetwork/java/codeconventions-150003.pdf

(Personally I would have just used an if / else if construct in this case, and not a switch at all.)

Copy link
Member Author

@gunterze gunterze Jun 30, 2017 •
edited

Choose a reason for hiding this comment

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

added the break and removed the duplicated lines...

}

if (StringUtils.containsWildCard(keyVal)) {
Pattern pattern = StringUtils.compilePattern(keyVal, ignoreCase);
for (String val : vals) {
Expand All @@ -2966,6 +2976,12 @@ private boolean matches(String privateCreator, int tag, VR vr,
return true;
else
continue;
if (dateRange != null)
if (dateRange.contains(
vr.toDate(val, getTimeZone(), 0, false, null, new DatePrecision())))
return true;
else
continue;
if (vr == VR.PN)
val = new PersonName(val).toString();
if (ignoreCase ? keyVal.equalsIgnoreCase(val)
Expand Down