Moti Asayag has posted comments on this change.

Change subject: tools: CidrValidation Utils
......................................................................


Patch Set 12:

(6 comments)

http://gerrit.ovirt.org/#/c/32539/12/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/validation/CidrValidator.java
File 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/validation/CidrValidator.java:

Line 6: 
Line 7:     private static CidrValidator cidrValidator = new CidrValidator();
Line 8: 
Line 9:     private CidrValidator() {
Line 10:     };
no need for ;
Line 11: 
Line 12:     public static CidrValidator getInstance() {
Line 13:         return cidrValidator;
Line 14:     }


Line 26:      *
Line 27:      * @return true if correct format, false otherwise.
Line 28:      */
Line 29:     public boolean isCidrFormatValid(String cidr) {
Line 30:         return (cidr != null && 
cidr.matches(ValidationUtils.CIDR_FORMAT_PATTERN));
no need for surrounding brackets
Line 31:     }
Line 32: 
Line 33:     /***
Line 34:      * check if CIDR represent valid network add


Line 31:     }
Line 32: 
Line 33:     /***
Line 34:      * check if CIDR represent valid network add
Line 35:      * <p>
<p> is redundant in this context
Line 36:      *
Line 37:      * @param cidr
Line 38:      *            - in correct format, please use the following 
function first: @see
Line 39:      *            {@link CidrValidator#isCidrFormatValid(String)}


Line 34:      * check if CIDR represent valid network add
Line 35:      * <p>
Line 36:      *
Line 37:      * @param cidr
Line 38:      *            - in correct format, please use the following 
function first: @see
the leading dash is not required
Line 39:      *            {@link CidrValidator#isCidrFormatValid(String)}
Line 40:      * @return true if valid CIDR ,false otherwise
Line 41:      */
Line 42:     public boolean isCidrNetworkAddressValid(String cidr) {


Line 53:         for (int index = 3; index > -1; index--) {
Line 54:             temp = Integer.parseInt(subAdd[3 - index]);
Line 55:             temp <<= (index * 8);
Line 56:             output |= temp;
Line 57:         }
please add a space line after closing curly bracket
Line 58:         return output;
Line 59:     }
Line 60: 
Line 61:     private static boolean isNetworkAddress(long ip, int mask) {


http://gerrit.ovirt.org/#/c/32539/12/backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/validation/CidrValidatorTest.java
File 
backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/validation/CidrValidatorTest.java:

Line 25:     }
Line 26: 
Line 27:     @Test
Line 28:     public void checkCidrFormatValidation() {
Line 29:         assertEquals("Failed to validate CIDR's Format: " + cidr, 
validCidrFormatExpectedResult, 
CidrValidator.getInstance().isCidrFormatValid(cidr));//$NON-NLS-1$
please remove the //$NON-NLS-1$ from this file.

The code seems to compile fine without it, for example:
backend/manager/modules/common/src/test/java/org/ovirt/engine/core/common/utils/ListUtilsTest.java
Line 30:     }
Line 31: 
Line 32:     @Test
Line 33:     public void checkNetworkAddressValidation() {


-- 
To view, visit http://gerrit.ovirt.org/32539
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib1277dbc815953926fe1648350cd55cb75e1084a
Gerrit-PatchSet: 12
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eliraz Levi <el...@redhat.com>
Gerrit-Reviewer: Alona Kaplan <alkap...@redhat.com>
Gerrit-Reviewer: Eliraz Levi <el...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Yevgeny Zaspitsky <yzasp...@redhat.com>
Gerrit-Reviewer: automat...@ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to