Support Range Matching by Attributes.matches(Attributes keys,..) #125
Conversation
| @@ -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(); | |||
There was a problem hiding this comment.
I think this if is now handled in the switch below, so the if can be removed.
| case DA: | ||
| case DT: | ||
| case TM: | ||
| dateRange = toDateRange(keyVal, vr); |
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
added the break and removed the duplicated lines...
dcm4che/dcm4chee-arc-light#830