You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2021/06/24 09:58:26 UTC
[GitHub] [commons-net] hari-om-888 opened a new pull request #84: adding inclusiveHostCount as constructor parameter
hari-om-888 opened a new pull request #84:
URL: https://github.com/apache/commons-net/pull/84
`SubnetUtils` classes is right now initialised with `inclusiveHostCount` as false by default.
Generally we are either interested into inclusiveHostCount being false or true since the inception of use case. We do not change it after the object of `SubnetUtils` have been created.
Also if `inclusiveHostCount` is supported as constructor parameters, it makes the use case of `inclusiveHostCount` more verbose. Otherwise it is very difficult for new person to find out what if such a parameter exist let alone default behaviour of it being false.
This pr adds constructors which support `inclusiveHostCount` as constructor parameters.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] sebbASF commented on pull request #84: adding inclusiveHostCount as constructor parameter of SubnetUtils
Posted by GitBox <gi...@apache.org>.
sebbASF commented on pull request #84:
URL: https://github.com/apache/commons-net/pull/84#issuecomment-886593174
Thanks for the suggestion anyway
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] sebbASF commented on a change in pull request #84: adding inclusiveHostCount as constructor parameter of SubnetUtils
Posted by GitBox <gi...@apache.org>.
sebbASF commented on a change in pull request #84:
URL: https://github.com/apache/commons-net/pull/84#discussion_r676496432
##########
File path: src/main/java/org/apache/commons/net/util/SubnetUtils.java
##########
@@ -331,6 +331,33 @@ public SubnetUtils(final String address, final String mask) {
this.broadcast = this.network | ~this.netmask;
}
+ /**
+ * Constructor that takes a CIDR-notation string, e.g. "192.168.0.1/16", a dotted decimal mask and and flag to
+ * return value of {@link SubnetInfo#getAddressCount()}
+ * @param cidrNotation A CIDR-notation string, e.g. "192.168.0.1/16"
+ * @param inclusiveHostCount A boolean value e.g. true
+ * @throws IllegalArgumentException if the parameter is invalid,
+ * i.e. does not match n.n.n.n/m where n=1-3 decimal digits, m = 1-2 decimal digits in range 0-32
+ */
+ public SubnetUtils(final String cidrNotation, final boolean inclusiveHostCount) {
Review comment:
Agreed.
It would have been better to add these variants instead of creating the setter, but that is done now, and it's not possible to make the class final now without breaking the API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] garydgregory commented on a change in pull request #84: adding inclusiveHostCount as constructor parameter of SubnetUtils
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #84:
URL: https://github.com/apache/commons-net/pull/84#discussion_r659233294
##########
File path: src/main/java/org/apache/commons/net/util/SubnetUtils.java
##########
@@ -331,6 +331,33 @@ public SubnetUtils(final String address, final String mask) {
this.broadcast = this.network | ~this.netmask;
}
+ /**
+ * Constructor that takes a CIDR-notation string, e.g. "192.168.0.1/16", a dotted decimal mask and and flag to
+ * return value of {@link SubnetInfo#getAddressCount()}
+ * @param cidrNotation A CIDR-notation string, e.g. "192.168.0.1/16"
+ * @param inclusiveHostCount A boolean value e.g. true
+ * @throws IllegalArgumentException if the parameter is invalid,
+ * i.e. does not match n.n.n.n/m where n=1-3 decimal digits, m = 1-2 decimal digits in range 0-32
+ */
+ public SubnetUtils(final String cidrNotation, final boolean inclusiveHostCount) {
Review comment:
Feels superflous since there is a setter method. We do not want, IMO, to add more contructor variants for this and that setting. So -1 from me. Let's see what others think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] sebbASF closed pull request #84: adding inclusiveHostCount as constructor parameter of SubnetUtils
Posted by GitBox <gi...@apache.org>.
sebbASF closed pull request #84:
URL: https://github.com/apache/commons-net/pull/84
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-net] hari-om-888 commented on a change in pull request #84: adding inclusiveHostCount as constructor parameter of SubnetUtils
Posted by GitBox <gi...@apache.org>.
hari-om-888 commented on a change in pull request #84:
URL: https://github.com/apache/commons-net/pull/84#discussion_r657808082
##########
File path: src/main/java/org/apache/commons/net/util/SubnetUtils.java
##########
@@ -331,6 +331,33 @@ public SubnetUtils(final String address, final String mask) {
this.broadcast = this.network | ~this.netmask;
}
+ /**
+ * Constructor that takes a CIDR-notation string, e.g. "192.168.0.1/16", a dotted decimal mask and and flag to
+ * return value of {@link SubnetInfo#getAddressCount()}
+ * @param cidrNotation A CIDR-notation string, e.g. "192.168.0.1/16"
+ * @param inclusiveHostCount A boolean value e.g. true
+ * @throws IllegalArgumentException if the parameter is invalid,
+ * i.e. does not match n.n.n.n/m where n=1-3 decimal digits, m = 1-2 decimal digits in range 0-32
+ */
+ public SubnetUtils(final String cidrNotation, final boolean inclusiveHostCount) {
Review comment:
using constructor overloading and setting inclusiveHost count.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org