Use final

- Merge if's with the same return value - Refactor magic numbers
apache · Dec 27, 2024 · 5198fc9 · 5198fc9
1 parent 75cdd06
commit 5198fc9
Show file tree

Hide file tree

Showing 11 changed files with 28 additions and 108 deletions.
diff --git a/src/main/java/org/apache/commons/validator/CreditCardValidator.java b/src/main/java/org/apache/commons/validator/CreditCardValidator.java
@@ -209,10 +209,7 @@ public void addAllowedCardType(final CreditCardType type){
      * @return Whether the card number is valid.
      */
     public boolean isValid(final String card) {
-        if (card == null || card.length() < 13 || card.length() > 19) {
-            return false;
-        }
-        if (!luhnCheck(card)) {
+        if (card == null || card.length() < 13 || card.length() > 19 || !luhnCheck(card)) {
             return false;
         }
         for (final Object cardType : cardTypes) {

diff --git a/src/main/java/org/apache/commons/validator/EmailValidator.java b/src/main/java/org/apache/commons/validator/EmailValidator.java
@@ -110,10 +110,7 @@ protected boolean isValidDomain(final String domain) {
             symbolic = DOMAIN_PATTERN.matcher(domain).matches();
         }
 
-        if (!symbolic) {
-            return false;
-        }
-        if (!isValidSymbolicDomain(domain)) {
+        if (!symbolic || !isValidSymbolicDomain(domain)) {
             return false;
         }
 
@@ -181,10 +178,7 @@ protected boolean isValidSymbolicDomain(String domain) {
         }
 
         final String tld = domainSegment[len - 1];
-        if (tld.length() <= 1) {
-            return false;
-        }
-        if (! TLD_PATTERN.matcher(tld).matches()) {
+        if (tld.length() <= 1 || ! TLD_PATTERN.matcher(tld).matches()) {
             return false;
         }
 

diff --git a/src/main/java/org/apache/commons/validator/UrlValidator.java b/src/main/java/org/apache/commons/validator/UrlValidator.java
@@ -78,6 +78,10 @@
 @Deprecated
 public class UrlValidator implements Serializable {
 
+    private static final int TOP_LEVEL_MAX_LEN = 4;
+
+    private static final int TOP_LEVEL_MIN_LEN = 2;
+
     private static final long serialVersionUID = 24137157400029593L;
 
     /**
@@ -259,28 +263,13 @@ protected int countToken(final String token, final String target) {
      * @return true if the URL is valid.
      */
     public boolean isValid(final String value) {
-        if (value == null) {
-            return false;
-        }
-        if (!LEGAL_ASCII_PATTERN.matcher(value).matches()) {
+        if (value == null || !LEGAL_ASCII_PATTERN.matcher(value).matches()) {
            return false;
         }
 
         // Check the whole url address structure
         final Matcher urlMatcher = URL_PATTERN.matcher(value);
-        if (!urlMatcher.matches()) {
-            return false;
-        }
-
-        if (!isValidScheme(urlMatcher.group(PARSE_URL_SCHEME))) {
-            return false;
-        }
-
-        if (!isValidAuthority(urlMatcher.group(PARSE_URL_AUTHORITY))) {
-            return false;
-        }
-
-        if (!isValidPath(urlMatcher.group(PARSE_URL_PATH))) {
+        if (!urlMatcher.matches() || !isValidScheme(urlMatcher.group(PARSE_URL_SCHEME)) || !isValidAuthority(urlMatcher.group(PARSE_URL_AUTHORITY)) || !isValidPath(urlMatcher.group(PARSE_URL_PATH))) {
             return false;
         }
 
@@ -355,17 +344,12 @@ protected boolean isValidAuthority(final String authority) {
                 }
             }
             final String topLevel = domainSegment[segmentCount - 1];
-            if (topLevel.length() < 2 || topLevel.length() > 4) { // CHECKSTYLE IGNORE MagicNumber (deprecated code)
-                return false;
-            }
 
-            // First letter of top level must be a alpha
-            if (!ALPHA_PATTERN.matcher(topLevel.substring(0, 1)).matches()) {
-                return false;
-            }
 
+            // First letter of top level must be a alpha
             // Make sure there's a host name preceding the authority.
-            if (segmentCount < 2) {
+            if (topLevel.length() < TOP_LEVEL_MIN_LEN || topLevel.length() > TOP_LEVEL_MAX_LEN || !ALPHA_PATTERN.matcher(topLevel.substring(0, 1)).matches()
+                    || segmentCount < 2) {
                 return false;
             }
         }
@@ -406,11 +390,7 @@ protected boolean isValidFragment(final String fragment) {
      * @return true if path is valid.
      */
     protected boolean isValidPath(final String path) {
-        if (path == null) {
-            return false;
-        }
-
-        if (!PATH_PATTERN.matcher(path).matches()) {
+        if (path == null || !PATH_PATTERN.matcher(path).matches()) {
             return false;
         }
 
@@ -450,15 +430,7 @@ protected boolean isValidQuery(final String query) {
      * @return true if valid.
      */
     protected boolean isValidScheme(final String scheme) {
-        if (scheme == null) {
-            return false;
-        }
-
-        if (!SCHEME_PATTERN.matcher(scheme).matches()) {
-            return false;
-        }
-
-        if (options.isOff(ALLOW_ALL_SCHEMES) && !allowedSchemes.contains(scheme)) {
+        if (scheme == null || !SCHEME_PATTERN.matcher(scheme).matches() || options.isOff(ALLOW_ALL_SCHEMES) && !allowedSchemes.contains(scheme)) {
             return false;
         }
 

diff --git a/src/main/java/org/apache/commons/validator/routines/AbstractFormatValidator.java b/src/main/java/org/apache/commons/validator/routines/AbstractFormatValidator.java
@@ -191,10 +191,7 @@ public boolean isValid(final String value, final String pattern) {
     protected Object parse(final String value, final Format formatter) {
         final ParsePosition pos = new ParsePosition(0);
         Object parsedValue = formatter.parseObject(value, pos);
-        if (pos.getErrorIndex() > -1) {
-            return null;
-        }
-        if (isStrict() && pos.getIndex() < value.length()) {
+        if (pos.getErrorIndex() > -1 || isStrict() && pos.getIndex() < value.length()) {
             return null;
         }
         if (parsedValue != null) {

diff --git a/src/main/java/org/apache/commons/validator/routines/EmailValidator.java b/src/main/java/org/apache/commons/validator/routines/EmailValidator.java
@@ -173,10 +173,7 @@ public EmailValidator(final boolean allowLocal, final boolean allowTld, final Do
      * @return true if the email address is valid.
      */
     public boolean isValid(final String email) {
-        if (email == null) {
-            return false;
-        }
-        if (email.endsWith(".")) { // check this first - it's cheap!
+        if ((email == null) || email.endsWith(".")) { // check this first - it's cheap!
             return false;
         }
         // Check the whole email address structure

diff --git a/src/main/java/org/apache/commons/validator/routines/FloatValidator.java b/src/main/java/org/apache/commons/validator/routines/FloatValidator.java
@@ -197,18 +197,12 @@ protected Object processParsedValue(final Object value, final Format formatter)
         final double doubleValue = ((Number) value).doubleValue();
 
         if (doubleValue > 0) {
-            if (doubleValue < Float.MIN_VALUE) {
-                return null;
-            }
-            if (doubleValue > Float.MAX_VALUE) {
+            if (doubleValue < Float.MIN_VALUE || doubleValue > Float.MAX_VALUE) {
                 return null;
             }
         } else if (doubleValue < 0) {
             final double posDouble = doubleValue * -1;
-            if (posDouble < Float.MIN_VALUE) {
-                return null;
-            }
-            if (posDouble > Float.MAX_VALUE) {
+            if (posDouble < Float.MIN_VALUE || posDouble > Float.MAX_VALUE) {
                 return null;
             }
         }

diff --git a/src/main/java/org/apache/commons/validator/routines/IBANValidator.java b/src/main/java/org/apache/commons/validator/routines/IBANValidator.java
@@ -390,7 +390,7 @@ public Validator setValidator(final Validator validator) {
      * @return {@link IBANValidatorStatus} for validation
      * @since 1.10.0
      */
-    public IBANValidatorStatus validate(String code) {
+    public IBANValidatorStatus validate(final String code) {
         final Validator formatValidator = getValidator(code);
         if (formatValidator == null) {
             return IBANValidatorStatus.UNKNOWN_COUNTRY;

diff --git a/src/main/java/org/apache/commons/validator/routines/InetAddressValidator.java b/src/main/java/org/apache/commons/validator/routines/InetAddressValidator.java
@@ -110,10 +110,7 @@ public boolean isValidInet4Address(final String inet4Address) {
             } catch (final NumberFormatException e) {
                 return false;
             }
-            if (iIpSegment > IPV4_MAX_OCTET_VALUE) {
-                return false;
-            }
-            if (ipSegment.length() > 1 && ipSegment.startsWith("0")) {
+            if (iIpSegment > IPV4_MAX_OCTET_VALUE || ipSegment.length() > 1 && ipSegment.startsWith("0")) {
                 return false;
             }
         }
@@ -145,12 +142,9 @@ public boolean isValidInet6Address(String inet6Address) {
         }
         // remove zone-id
         parts = parts[0].split("%", -1);
-        if (parts.length > 2) {
-            return false;
-        }
         // The id syntax is implementation independent, but it presumably cannot allow:
         // whitespace, '/' or '%'
-        if (parts.length == 2 && !ID_CHECK_PATTERN.matcher(parts[1]).matches()) {
+        if ((parts.length > 2) || (parts.length == 2 && !ID_CHECK_PATTERN.matcher(parts[1]).matches())) {
             return false; // invalid id
         }
         inet6Address = parts[0];

diff --git a/src/main/java/org/apache/commons/validator/routines/UrlValidator.java b/src/main/java/org/apache/commons/validator/routines/UrlValidator.java
@@ -377,23 +377,12 @@ public boolean isValid(final String value) {
         if ("file".equals(scheme) && GenericValidator.isBlankOrNull(authority)) { // Special case - file: allows an empty authority
             return true; // this is a local file - nothing more to do here
         }
-        if ("file".equals(scheme) && authority != null && authority.contains(":")) {
-            return false;
-        }
         // Validate the authority
-        if (!isValidAuthority(authority)) {
-            return false;
-        }
-
-        if (!isValidPath(uri.getRawPath())) {
+        if ("file".equals(scheme) && authority != null && authority.contains(":") || !isValidAuthority(authority)) {
             return false;
         }
 
-        if (!isValidQuery(uri.getRawQuery())) {
-            return false;
-        }
-
-        if (!isValidFragment(uri.getRawFragment())) {
+        if (!isValidPath(uri.getRawPath()) || !isValidQuery(uri.getRawQuery()) || !isValidFragment(uri.getRawFragment())) {
             return false;
         }
 
@@ -486,11 +475,7 @@ protected boolean isValidFragment(final String fragment) {
      * @return true if path is valid.
      */
     protected boolean isValidPath(final String path) {
-        if (path == null) {
-            return false;
-        }
-
-        if (!PATH_PATTERN.matcher(path).matches()) {
+        if (path == null || !PATH_PATTERN.matcher(path).matches()) {
             return false;
         }
 
@@ -536,15 +521,8 @@ protected boolean isValidQuery(final String query) {
      * @return true if valid.
      */
     protected boolean isValidScheme(final String scheme) {
-        if (scheme == null) {
-            return false;
-        }
-
-        if (!SCHEME_PATTERN.matcher(scheme).matches()) {
-            return false;
-        }
-
-        if (isOff(ALLOW_ALL_SCHEMES) && !allowedSchemes.contains(scheme.toLowerCase(Locale.ENGLISH))) {
+        if (scheme == null || !SCHEME_PATTERN.matcher(scheme).matches()
+                || isOff(ALLOW_ALL_SCHEMES) && !allowedSchemes.contains(scheme.toLowerCase(Locale.ENGLISH))) {
             return false;
         }
 

diff --git a/src/main/java/org/apache/commons/validator/routines/checkdigit/ModulusTenCheckDigit.java b/src/main/java/org/apache/commons/validator/routines/checkdigit/ModulusTenCheckDigit.java
@@ -183,10 +183,7 @@ public ModulusTenCheckDigit(final int[] positionWeight, final boolean useRightPo
      */
     @Override
     public boolean isValid(final String code) {
-        if (GenericValidator.isBlankOrNull(code)) {
-            return false;
-        }
-        if (!Character.isDigit(code.charAt(code.length() - 1))) {
+        if (GenericValidator.isBlankOrNull(code) || !Character.isDigit(code.charAt(code.length() - 1))) {
             return false;
         }
         return super.isValid(code);

diff --git a/src/test/java/org/apache/commons/validator/routines/IBANValidatorTest.java b/src/test/java/org/apache/commons/validator/routines/IBANValidatorTest.java
@@ -473,7 +473,7 @@ public void testValid(final String iban) {
 
     @ParameterizedTest
     @MethodSource
-    public void validateIbanStatuses(String iban, IBANValidatorStatus expectedStatus) {
+    public void validateIbanStatuses(final String iban, final IBANValidatorStatus expectedStatus) {
         assertEquals(expectedStatus, IBANValidator.getInstance().validate(iban));
     }