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/03/03 17:51:13 UTC
[GitHub] [commons-lang] arturobernalg opened a new pull request #727: LANG-1644 - Check if number is hexadecimal
arturobernalg opened a new pull request #727:
URL: https://github.com/apache/commons-lang/pull/727
Checks whether the given String is a hex number.
* NumberUtils.isHexNumber(null)) = false
* NumberUtils.isHexNumber("")) = false
* NumberUtils.isHexNumber("0x12345678")) = true
* NumberUtils.isHexNumber("0x7fffffffffffffff")) = true
* NumberUtils.isHexNumber("0x7FFFFFFFFFFFFFFF")) = true
* NumberUtils.isHexNumber("5D0")) = true
* NumberUtils.isHexNumber("0x")) = false
----------------------------------------------------------------
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-lang] aherbert commented on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
aherbert commented on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-790912955
The value `5D0` may be valid hex digits but what are you going to do with that fact? If you want to create a number with it then nothing in NumberUtils or the Java language will make a number from that string value. They all require that the string is marked with a prefix to identify it should be parsed as hex.
For example the following is valid hex `444` but if you parse it as `0x444` and `444` you get different numbers.
So what is the use case for a method that identifies if a string has only valid hexidecimal digits and optionally a minus and a hex prefix?
----------------------------------------------------------------
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-lang] garydgregory commented on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
garydgregory commented on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789936882
You should really run a local build before creating the PR as this PR fails the build.
----------------------------------------------------------------
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-lang] arturobernalg commented on a change in pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#discussion_r587531376
##########
File path: src/main/java/org/apache/commons/lang3/math/NumberUtils.java
##########
@@ -1715,6 +1715,50 @@ public static boolean isCreatable(final String str) {
return !allowSigns && foundDigit;
}
+ /**
+ * Checks whether the given String is a hex number.
+ *
+ * <p>Valid parameter include hexadecimal marked with the {@code 0x} or
+ * {@code 0X} qualifier.</p>
+ *
+ * <p>{@code Null} and empty String will return {@code false}.</p>
+ *
+ * <pre>
+ * NumberUtils.isHexNumber(null)) = false
+ * NumberUtils.isHexNumber("")) = false
+ * NumberUtils.isHexNumber("0x12345678")) = true
+ * NumberUtils.isHexNumber("0x7fffffffffffffff")) = true
+ * NumberUtils.isHexNumber("0x7FFFFFFFFFFFFFFF")) = true
+ * NumberUtils.isHexNumber("5D0")) = true
+ * NumberUtils.isHexNumber("0x")) = false
+ * </pre>
+ *
+ * @param str the String to check.
+ * @return {@code true} if the string is a hex number.
+ * @since 3.12.1
+ */
+ public static boolean isHexNumber(final String str) {
+ if (StringUtils.isEmpty(str)) {
+ return false;
+ }
+ final char[] chars = str.toCharArray();
+ final int length = chars.length;
+ int i = 1;
+ if (chars[i] == 'x' || chars[i] == 'X') {
+ i = 2;
+ if (i == length) {
+ return false;
+ }
+ }
+ for (; i < chars.length; i++) {
Review comment:
Hi @garydgregory
I've trying use Range and i think it's more clean check the one by one the char. With range should look like:
` final Range<Integer> range = Range.between(0, 15);
for (; pfxLen < chars.length; pfxLen++) {
if (!range.contains(Character.getNumericValue(chars[pfxLen]))) {
return false;
}
}`
----------------------------------------------------------------
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-lang] coveralls edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
[](https://coveralls.io/builds/37655940)
Coverage increased (+0.005%) to 94.919% when pulling **3b6abe281e5173c2f55b5262ac5ccaa210108552 on arturobernalg:feature/LANG-1644** into **b59c7bfe249fc2d278f43492daf726b53eb8b28d on apache:master**.
----------------------------------------------------------------
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-lang] coveralls edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
----------------------------------------------------------------
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-lang] coveralls edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
[](https://coveralls.io/builds/37677908)
Coverage increased (+0.005%) to 94.919% when pulling **f8a09c4337de44e676f68f9c43f1c2846be2adfa on arturobernalg:feature/LANG-1644** into **195ecefc8d280d03eea601dfa68b58d0494237b2 on apache:master**.
----------------------------------------------------------------
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-lang] arturobernalg commented on a change in pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#discussion_r587531376
##########
File path: src/main/java/org/apache/commons/lang3/math/NumberUtils.java
##########
@@ -1715,6 +1715,50 @@ public static boolean isCreatable(final String str) {
return !allowSigns && foundDigit;
}
+ /**
+ * Checks whether the given String is a hex number.
+ *
+ * <p>Valid parameter include hexadecimal marked with the {@code 0x} or
+ * {@code 0X} qualifier.</p>
+ *
+ * <p>{@code Null} and empty String will return {@code false}.</p>
+ *
+ * <pre>
+ * NumberUtils.isHexNumber(null)) = false
+ * NumberUtils.isHexNumber("")) = false
+ * NumberUtils.isHexNumber("0x12345678")) = true
+ * NumberUtils.isHexNumber("0x7fffffffffffffff")) = true
+ * NumberUtils.isHexNumber("0x7FFFFFFFFFFFFFFF")) = true
+ * NumberUtils.isHexNumber("5D0")) = true
+ * NumberUtils.isHexNumber("0x")) = false
+ * </pre>
+ *
+ * @param str the String to check.
+ * @return {@code true} if the string is a hex number.
+ * @since 3.12.1
+ */
+ public static boolean isHexNumber(final String str) {
+ if (StringUtils.isEmpty(str)) {
+ return false;
+ }
+ final char[] chars = str.toCharArray();
+ final int length = chars.length;
+ int i = 1;
+ if (chars[i] == 'x' || chars[i] == 'X') {
+ i = 2;
+ if (i == length) {
+ return false;
+ }
+ }
+ for (; i < chars.length; i++) {
Review comment:
Hi @garydgregory
I've trying use Range and i think it's more clean check the one by one the char. With range should look like:
`
final Range<Integer> range = Range.between(0, 15);
for (; pfxLen < chars.length; pfxLen++) {
if (!range.contains(Character.getNumericValue(chars[pfxLen]))) {
return false;
}
}
`
----------------------------------------------------------------
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-lang] arturobernalg commented on a change in pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#discussion_r587306048
##########
File path: src/main/java/org/apache/commons/lang3/math/NumberUtils.java
##########
@@ -1715,6 +1715,50 @@ public static boolean isCreatable(final String str) {
return !allowSigns && foundDigit;
}
+ /**
+ * Checks whether the given String is a hex number.
+ *
+ * <p>Valid parameter include hexadecimal marked with the {@code 0x} or
+ * {@code 0X} qualifier.</p>
+ *
+ * <p>{@code Null} and empty String will return {@code false}.</p>
+ *
+ * <pre>
+ * NumberUtils.isHexNumber(null)) = false
+ * NumberUtils.isHexNumber("")) = false
+ * NumberUtils.isHexNumber("0x12345678")) = true
+ * NumberUtils.isHexNumber("0x7fffffffffffffff")) = true
+ * NumberUtils.isHexNumber("0x7FFFFFFFFFFFFFFF")) = true
+ * NumberUtils.isHexNumber("5D0")) = true
+ * NumberUtils.isHexNumber("0x")) = false
+ * </pre>
+ *
+ * @param str the String to check.
+ * @return {@code true} if the string is a hex number.
+ * @since 3.12.1
+ */
+ public static boolean isHexNumber(final String str) {
+ if (StringUtils.isEmpty(str)) {
+ return false;
+ }
+ final char[] chars = str.toCharArray();
+ final int length = chars.length;
+ int i = 1;
+ if (chars[i] == 'x' || chars[i] == 'X') {
+ i = 2;
+ if (i == length) {
+ return false;
+ }
+ }
+ for (; i < chars.length; i++) {
Review comment:
HI @garydgregory
What do you think about the Range change?
----------------------------------------------------------------
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-lang] arturobernalg commented on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-791194290
Hi @aherbert
----------------------------------------------------------------
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-lang] arturobernalg commented on a change in pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#discussion_r586668513
##########
File path: src/main/java/org/apache/commons/lang3/math/NumberUtils.java
##########
@@ -1715,6 +1715,50 @@ public static boolean isCreatable(final String str) {
return !allowSigns && foundDigit;
}
+ /**
+ * Checks whether the given String is a hex number.
+ *
+ * <p>Valid parameter include hexadecimal marked with the {@code 0x} or
+ * {@code 0X} qualifier.</p>
+ *
+ * <p>{@code Null} and empty String will return {@code false}.</p>
+ *
+ * <pre>
+ * NumberUtils.isHexNumber(null)) = false
+ * NumberUtils.isHexNumber("")) = false
+ * NumberUtils.isHexNumber("0x12345678")) = true
+ * NumberUtils.isHexNumber("0x7fffffffffffffff")) = true
+ * NumberUtils.isHexNumber("0x7FFFFFFFFFFFFFFF")) = true
+ * NumberUtils.isHexNumber("5D0")) = true
+ * NumberUtils.isHexNumber("0x")) = false
+ * </pre>
+ *
+ * @param str the String to check.
+ * @return {@code true} if the string is a hex number.
+ * @since 3.12.1
+ */
+ public static boolean isHexNumber(final String str) {
+ if (StringUtils.isEmpty(str)) {
+ return false;
+ }
+ final char[] chars = str.toCharArray();
+ final int length = chars.length;
+ int i = 1;
+ if (chars[i] == 'x' || chars[i] == 'X') {
+ i = 2;
+ if (i == length) {
+ return false;
+ }
+ }
+ for (; i < chars.length; i++) {
Review comment:
Hi @garydgregory
don't you think this may be complicated the code? are you thinking in a global solution, implement in isCreatable perhaps?
----------------------------------------------------------------
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-lang] coveralls edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
[](https://coveralls.io/builds/37678592)
Coverage decreased (-0.002%) to 94.912% when pulling **15e31d99d7b74ceb1a5d0506cdd8ecb3c3ab5122 on arturobernalg:feature/LANG-1644** into **615410d3fd90f91f422f0f0037b7d64df2cac36c on apache:master**.
----------------------------------------------------------------
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-lang] coveralls commented on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
[](https://coveralls.io/builds/37627057)
Coverage increased (+0.004%) to 94.918% when pulling **f49fa83272cb326d50dac979e754140a96188223 on arturobernalg:feature/LANG-1644** into **b59c7bfe249fc2d278f43492daf726b53eb8b28d on apache:master**.
----------------------------------------------------------------
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
Re: [GitHub] [commons-lang] aherbert commented on pull request #727:
LANG-1644 - Check if number is hexadecimal
Posted by Alex Herbert <al...@gmail.com>.
Sorry, the previous simple regex was incorrect. It should have at least 1
character after the x:
0[xX][0-9a-fA-F]{1,16}
Anyway, parsing a number by code or regex has a lot of cases to consider
and the intent should be documented.
On Thu, 4 Mar 2021 at 00:47, Alex Herbert <al...@gmail.com> wrote:
> It also makes me ask if we are adding this then why not methods to query
> the other Java formats for literals:
>
> Binary = 0b...
> Octal = 0...
> Hex = 0x...
>
> isBinaryLiteral()
> isOctalLiteral()
> isHexLiteral()
>
> It also assumes that the number is an integer. But the binary literal
> formats cover floating point too, e.g.
>
> float f = -0x0.1abcdefp-12f;
>
> Parsing this is far more complex than detecting the hex characters in a
> regex type manner: 0[xX][0-9a-fA-F]{0,16}.
>
>
> On Thu, 4 Mar 2021 at 00:31, Gary Gregory <ga...@gmail.com> wrote:
>
>> I am not convinced this belong in Lang. Maybe in a Math module? Maybe in
>> Text?
>>
>> Gary
>>
>> On Wed, Mar 3, 2021, 19:29 GitBox <gi...@apache.org> wrote:
>>
>> >
>> > aherbert commented on pull request #727:
>> > URL:
>> > https://github.com/apache/commons-lang/pull/727#issuecomment-790185530
>> >
>> >
>> > Is the intention that this should match `isCreateable(String)` for
>> all
>> > valid hex numbers and return `true` for all hex cases where a `Number`
>> is
>> > returned from `createNumber(String)`?
>> > ```java
>> > public static boolean isCreatable(final String str)
>> > public static Number createNumber(final String str)
>> > ```
>> > If so then you should support the `#` character (and all the other
>> hex
>> > prefixes) and update the javadoc to reflect the intention.
>> >
>> > I would then update the test to use all the hex cases from the tests
>> > for `createNumber` and `isCreateable` to ensure it returns `true` when
>> > those methods succeed.
>> >
>> > Currently your test only covers a range of `int` and `long` values,
>> no
>> > hex variants and does not specify whether BigInteger values are also
>> `true`.
>> >
>> > It will also fail on valid hex numbers such as `0x1L`.
>> >
>> >
>> >
>> >
>> > ----------------------------------------------------------------
>> > 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
>> >
>> >
>> >
>>
>
Re: [GitHub] [commons-lang] aherbert commented on pull request #727:
LANG-1644 - Check if number is hexadecimal
Posted by Alex Herbert <al...@gmail.com>.
It also makes me ask if we are adding this then why not methods to query
the other Java formats for literals:
Binary = 0b...
Octal = 0...
Hex = 0x...
isBinaryLiteral()
isOctalLiteral()
isHexLiteral()
It also assumes that the number is an integer. But the binary literal
formats cover floating point too, e.g.
float f = -0x0.1abcdefp-12f;
Parsing this is far more complex than detecting the hex characters in a
regex type manner: 0[xX][0-9a-fA-F]{0,16}.
On Thu, 4 Mar 2021 at 00:31, Gary Gregory <ga...@gmail.com> wrote:
> I am not convinced this belong in Lang. Maybe in a Math module? Maybe in
> Text?
>
> Gary
>
> On Wed, Mar 3, 2021, 19:29 GitBox <gi...@apache.org> wrote:
>
> >
> > aherbert commented on pull request #727:
> > URL:
> > https://github.com/apache/commons-lang/pull/727#issuecomment-790185530
> >
> >
> > Is the intention that this should match `isCreateable(String)` for all
> > valid hex numbers and return `true` for all hex cases where a `Number` is
> > returned from `createNumber(String)`?
> > ```java
> > public static boolean isCreatable(final String str)
> > public static Number createNumber(final String str)
> > ```
> > If so then you should support the `#` character (and all the other hex
> > prefixes) and update the javadoc to reflect the intention.
> >
> > I would then update the test to use all the hex cases from the tests
> > for `createNumber` and `isCreateable` to ensure it returns `true` when
> > those methods succeed.
> >
> > Currently your test only covers a range of `int` and `long` values, no
> > hex variants and does not specify whether BigInteger values are also
> `true`.
> >
> > It will also fail on valid hex numbers such as `0x1L`.
> >
> >
> >
> >
> > ----------------------------------------------------------------
> > 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
> >
> >
> >
>
Re: [GitHub] [commons-lang] aherbert commented on pull request #727:
LANG-1644 - Check if number is hexadecimal
Posted by Gary Gregory <ga...@gmail.com>.
I am not convinced this belong in Lang. Maybe in a Math module? Maybe in
Text?
Gary
On Wed, Mar 3, 2021, 19:29 GitBox <gi...@apache.org> wrote:
>
> aherbert commented on pull request #727:
> URL:
> https://github.com/apache/commons-lang/pull/727#issuecomment-790185530
>
>
> Is the intention that this should match `isCreateable(String)` for all
> valid hex numbers and return `true` for all hex cases where a `Number` is
> returned from `createNumber(String)`?
> ```java
> public static boolean isCreatable(final String str)
> public static Number createNumber(final String str)
> ```
> If so then you should support the `#` character (and all the other hex
> prefixes) and update the javadoc to reflect the intention.
>
> I would then update the test to use all the hex cases from the tests
> for `createNumber` and `isCreateable` to ensure it returns `true` when
> those methods succeed.
>
> Currently your test only covers a range of `int` and `long` values, no
> hex variants and does not specify whether BigInteger values are also `true`.
>
> It will also fail on valid hex numbers such as `0x1L`.
>
>
>
>
> ----------------------------------------------------------------
> 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-lang] aherbert commented on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
aherbert commented on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-790185530
Is the intention that this should match `isCreateable(String)` for all valid hex numbers and return `true` for all hex cases where a `Number` is returned from `createNumber(String)`?
```java
public static boolean isCreatable(final String str)
public static Number createNumber(final String str)
```
If so then you should support the `#` character (and all the other hex prefixes) and update the javadoc to reflect the intention.
I would then update the test to use all the hex cases from the tests for `createNumber` and `isCreateable` to ensure it returns `true` when those methods succeed.
Currently your test only covers a range of `int` and `long` values, no hex variants and does not specify whether BigInteger values are also `true`.
It will also fail on valid hex numbers such as `0x1L`.
----------------------------------------------------------------
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-lang] coveralls edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
[](https://coveralls.io/builds/39634611)
Coverage increased (+0.005%) to 94.932% when pulling **b47bcdde512df0904c1a2156d7089e06903db35e on arturobernalg:feature/LANG-1644** into **950ab130d616a67c39921b1d8c2c5b877ee8ad55 on apache:master**.
--
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-lang] arturobernalg commented on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789942178
> You should really run a local build before creating the PR as this PR fails the build.
Yep, my mistake. Sorry.
----------------------------------------------------------------
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-lang] aherbert commented on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
aherbert commented on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-790806626
Regarding `0x1L` the NumberUtils.createNumber and NumberUtils.isCreatable only supports the preferred type suffix on decimal numbers. You cannot specify a preferred `L` for hex integers. So this new method should match the existing behaviour of those methods. So you can ignore this.
I am not sure that allowing the number `5D0` to be `true` is valid. This would not be parsed or created as a Hex integer by any method in NumberUtils. `Integer.decode("5D0")` throws an exception. Thus a hex number should be identified with `0x`, `0X` or `#` with an optional sign prefix (see [LANG-1645] and #728) and thereafter only contain the digits `[0-9A-Fa-f]`.
This method then becomes a subset of `isCreateable` that will return true only if the `Number` created by `createNumber` is a mathematical integer.
----------------------------------------------------------------
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-lang] arturobernalg commented on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-790332374
> BigInteger
Hi @aherbert
Thank you for your comment. I've fix those cases that you mentioned and i've add more test.
Just one think about --> `0x1L`. I don't know what to do with that. If you execute the test `compareIsNumberWithCreateNumber("0x1L", true)`; will throw an exception. Same case with
`NumberUtils.isParsable("0x1L")
`
----------------------------------------------------------------
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-lang] coveralls edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
[](https://coveralls.io/builds/37661177)
Coverage decreased (-0.001%) to 94.913% when pulling **c07682d1cb51c50e1f6dbc0604a5bd6d1f772098 on arturobernalg:feature/LANG-1644** into **195ecefc8d280d03eea601dfa68b58d0494237b2 on apache:master**.
----------------------------------------------------------------
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-lang] arturobernalg edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-791194290
Hi @aherbert
I will follow your approach.
Regards
----------------------------------------------------------------
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-lang] arturobernalg edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-791194290
Hi @aherbert
So, basically the function only work in NumberUtils libs. All the idea of have a commons lib it will broke. Or make the private function.
Regards
----------------------------------------------------------------
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-lang] coveralls edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
[](https://coveralls.io/builds/37689076)
Coverage decreased (-0.007%) to 94.909% when pulling **4d1292421d3caabd9abe6ee32b71010e6b5c45f1 on arturobernalg:feature/LANG-1644** into **0cfc31b98778767cb2f8db93bc2f488abe058e2e on apache:master**.
----------------------------------------------------------------
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-lang] arturobernalg edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-791194290
Hi @aherbert
So, according with you the function should only work only in NumberUtils libs. All the idea of have a commons lib, used by anyone for any propouse, it will broke. For do that it's better to make it private
Regards
----------------------------------------------------------------
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-lang] arturobernalg commented on a change in pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#discussion_r587306048
##########
File path: src/main/java/org/apache/commons/lang3/math/NumberUtils.java
##########
@@ -1715,6 +1715,50 @@ public static boolean isCreatable(final String str) {
return !allowSigns && foundDigit;
}
+ /**
+ * Checks whether the given String is a hex number.
+ *
+ * <p>Valid parameter include hexadecimal marked with the {@code 0x} or
+ * {@code 0X} qualifier.</p>
+ *
+ * <p>{@code Null} and empty String will return {@code false}.</p>
+ *
+ * <pre>
+ * NumberUtils.isHexNumber(null)) = false
+ * NumberUtils.isHexNumber("")) = false
+ * NumberUtils.isHexNumber("0x12345678")) = true
+ * NumberUtils.isHexNumber("0x7fffffffffffffff")) = true
+ * NumberUtils.isHexNumber("0x7FFFFFFFFFFFFFFF")) = true
+ * NumberUtils.isHexNumber("5D0")) = true
+ * NumberUtils.isHexNumber("0x")) = false
+ * </pre>
+ *
+ * @param str the String to check.
+ * @return {@code true} if the string is a hex number.
+ * @since 3.12.1
+ */
+ public static boolean isHexNumber(final String str) {
+ if (StringUtils.isEmpty(str)) {
+ return false;
+ }
+ final char[] chars = str.toCharArray();
+ final int length = chars.length;
+ int i = 1;
+ if (chars[i] == 'x' || chars[i] == 'X') {
+ i = 2;
+ if (i == length) {
+ return false;
+ }
+ }
+ for (; i < chars.length; i++) {
Review comment:
HI @garydgregory
What do you think about the Range change?
----------------------------------------------------------------
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-lang] coveralls edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
[](https://coveralls.io/builds/37644604)
Coverage increased (+0.005%) to 94.919% when pulling **63baa3058d2f499f034c741975aeeb77e56ab2a9 on arturobernalg:feature/LANG-1644** into **b59c7bfe249fc2d278f43492daf726b53eb8b28d on apache:master**.
----------------------------------------------------------------
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-lang] arturobernalg edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-791194290
Hi @aherbert
So, according with you the function should only work only in NumberUtils libs. All the idea of have a commons lib, used by anyone for any propouse, it will broke. For do that it's better to make it private
Regards
----------------------------------------------------------------
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-lang] aherbert commented on a change in pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
aherbert commented on a change in pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#discussion_r586910342
##########
File path: src/main/java/org/apache/commons/lang3/math/NumberUtils.java
##########
@@ -1715,6 +1715,50 @@ public static boolean isCreatable(final String str) {
return !allowSigns && foundDigit;
}
+ /**
+ * Checks whether the given String is a hex number.
+ *
+ * <p>Valid parameter include hexadecimal marked with the {@code 0x} or
+ * {@code 0X} qualifier.</p>
+ *
+ * <p>{@code Null} and empty String will return {@code false}.</p>
+ *
+ * <pre>
+ * NumberUtils.isHexNumber(null)) = false
+ * NumberUtils.isHexNumber("")) = false
+ * NumberUtils.isHexNumber("0x12345678")) = true
+ * NumberUtils.isHexNumber("0x7fffffffffffffff")) = true
+ * NumberUtils.isHexNumber("0x7FFFFFFFFFFFFFFF")) = true
+ * NumberUtils.isHexNumber("5D0")) = true
+ * NumberUtils.isHexNumber("0x")) = false
+ * </pre>
+ *
+ * @param str the String to check.
+ * @return {@code true} if the string is a hex number.
+ * @since 3.12.1
+ */
+ public static boolean isHexNumber(final String str) {
+ if (StringUtils.isEmpty(str)) {
+ return false;
+ }
+ final char[] chars = str.toCharArray();
+ final int length = chars.length;
+ int i = 1;
+ if (chars[i] == 'x' || chars[i] == 'X') {
Review comment:
If chars is length 1 then this will throw an index out of bounds exception.
----------------------------------------------------------------
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-lang] coveralls edited a comment on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-789980845
[](https://coveralls.io/builds/37648699)
Coverage increased (+0.006%) to 94.921% when pulling **99b5d9860e21e2a30d5987daa4ee063250d1a2f5 on arturobernalg:feature/LANG-1644** into **b59c7bfe249fc2d278f43492daf726b53eb8b28d on apache:master**.
----------------------------------------------------------------
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-lang] arturobernalg commented on a change in pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on a change in pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#discussion_r587531376
##########
File path: src/main/java/org/apache/commons/lang3/math/NumberUtils.java
##########
@@ -1715,6 +1715,50 @@ public static boolean isCreatable(final String str) {
return !allowSigns && foundDigit;
}
+ /**
+ * Checks whether the given String is a hex number.
+ *
+ * <p>Valid parameter include hexadecimal marked with the {@code 0x} or
+ * {@code 0X} qualifier.</p>
+ *
+ * <p>{@code Null} and empty String will return {@code false}.</p>
+ *
+ * <pre>
+ * NumberUtils.isHexNumber(null)) = false
+ * NumberUtils.isHexNumber("")) = false
+ * NumberUtils.isHexNumber("0x12345678")) = true
+ * NumberUtils.isHexNumber("0x7fffffffffffffff")) = true
+ * NumberUtils.isHexNumber("0x7FFFFFFFFFFFFFFF")) = true
+ * NumberUtils.isHexNumber("5D0")) = true
+ * NumberUtils.isHexNumber("0x")) = false
+ * </pre>
+ *
+ * @param str the String to check.
+ * @return {@code true} if the string is a hex number.
+ * @since 3.12.1
+ */
+ public static boolean isHexNumber(final String str) {
+ if (StringUtils.isEmpty(str)) {
+ return false;
+ }
+ final char[] chars = str.toCharArray();
+ final int length = chars.length;
+ int i = 1;
+ if (chars[i] == 'x' || chars[i] == 'X') {
+ i = 2;
+ if (i == length) {
+ return false;
+ }
+ }
+ for (; i < chars.length; i++) {
Review comment:
Hi @garydgregory
I've trying use Range and i think it's more clean check the one by one the char. With range should look like:
```
final Range<Integer> range = Range.between(0, 15);
for (; pfxLen < chars.length; pfxLen++) {
if (!range.contains(Character.getNumericValue(chars[pfxLen]))) {
return false;
}
}
```
----------------------------------------------------------------
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-lang] garydgregory commented on a change in pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
garydgregory commented on a change in pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#discussion_r586658202
##########
File path: src/main/java/org/apache/commons/lang3/math/NumberUtils.java
##########
@@ -1715,6 +1715,50 @@ public static boolean isCreatable(final String str) {
return !allowSigns && foundDigit;
}
+ /**
+ * Checks whether the given String is a hex number.
+ *
+ * <p>Valid parameter include hexadecimal marked with the {@code 0x} or
+ * {@code 0X} qualifier.</p>
+ *
+ * <p>{@code Null} and empty String will return {@code false}.</p>
+ *
+ * <pre>
+ * NumberUtils.isHexNumber(null)) = false
+ * NumberUtils.isHexNumber("")) = false
+ * NumberUtils.isHexNumber("0x12345678")) = true
+ * NumberUtils.isHexNumber("0x7fffffffffffffff")) = true
+ * NumberUtils.isHexNumber("0x7FFFFFFFFFFFFFFF")) = true
+ * NumberUtils.isHexNumber("5D0")) = true
+ * NumberUtils.isHexNumber("0x")) = false
+ * </pre>
+ *
+ * @param str the String to check.
+ * @return {@code true} if the string is a hex number.
+ * @since 3.12.1
+ */
+ public static boolean isHexNumber(final String str) {
+ if (StringUtils.isEmpty(str)) {
+ return false;
+ }
+ final char[] chars = str.toCharArray();
+ final int length = chars.length;
+ int i = 1;
+ if (chars[i] == 'x' || chars[i] == 'X') {
+ i = 2;
+ if (i == length) {
+ return false;
+ }
+ }
+ for (; i < chars.length; i++) {
Review comment:
Would be a neat use of our Range class?
----------------------------------------------------------------
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-lang] arturobernalg commented on pull request #727: LANG-1644 - Check if number is hexadecimal
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on pull request #727:
URL: https://github.com/apache/commons-lang/pull/727#issuecomment-790832106
> Regarding `0x1L` the NumberUtils.createNumber and NumberUtils.isCreatable only supports the preferred type suffix on decimal numbers. You cannot specify a preferred `L` for hex integers. So this new method should match the existing behaviour of those methods. So you can ignore this.
>
> I am not sure that allowing the number `5D0` to be `true` is valid. This would not be parsed or created as a Hex integer by any method in NumberUtils. `Integer.decode("5D0")` throws an exception. Thus a hex number should be identified with `0x`, `0X` or `#` with an optional sign prefix (see [LANG-1645] and #728) and thereafter only contain the digits `[0-9A-Fa-f]`.
>
> This method then becomes a subset of `isCreateable` that will return true only if the `Number` created by `createNumber` is a mathematical integer.
HI @aherbert
Depends on what we want to achieve with the method. `5D0` it's a valid hex and IMO should be considered for the method and returned true.
----------------------------------------------------------------
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