commons-lang icon indicating copy to clipboard operation
commons-lang copied to clipboard

LANG-1644 - Check if number is hexadecimal

Open arturobernalg opened this issue 3 years ago • 9 comments

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

arturobernalg avatar Mar 03 '21 17:03 arturobernalg

You should really run a local build before creating the PR as this PR fails the build.

garydgregory avatar Mar 03 '21 17:03 garydgregory

You should really run a local build before creating the PR as this PR fails the build.

Yep, my mistake. Sorry.

arturobernalg avatar Mar 03 '21 18:03 arturobernalg

Coverage Status

Coverage increased (+0.005%) to 94.932% when pulling b47bcdde512df0904c1a2156d7089e06903db35e on arturobernalg:feature/LANG-1644 into 950ab130d616a67c39921b1d8c2c5b877ee8ad55 on apache:master.

coveralls avatar Mar 03 '21 19:03 coveralls

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)?

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.

aherbert avatar Mar 04 '21 00:03 aherbert

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")

arturobernalg avatar Mar 04 '21 06:03 arturobernalg

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.

aherbert avatar Mar 04 '21 17:03 aherbert

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.

arturobernalg avatar Mar 04 '21 18:03 arturobernalg

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?

aherbert avatar Mar 04 '21 20:03 aherbert

Hi @aherbert

I will follow your approach.

Regards

arturobernalg avatar Mar 05 '21 06:03 arturobernalg