Skip to main content
added 2 characters in body
Source Link
mdfst13
  • 22.4k
  • 6
  • 34
  • 70
for (int index = 0; index < integerText.length(); index++) {
    long digit = Long.parseLong(integerText.substring(index, index + 1)));
    nextInteger = nextInteger.add(BigInteger.valueOf(longdigit));
}
for (int index = 0; index < integerText.length(); index += Character.charCount(integerText.codePointAt(index))) {
    Long digit = Long.parseLong(integerText.substring(index, index + Character.charCount(integerText.codePointAt(index)))));
    nextInteger = nextInteger.add(BigInteger.valueOf(longdigit));
}
for (int index = 0; index < integerText.length(); index++) {
    long digit = Long.parseLong(integerText.substring(index, index + 1)));
    nextInteger = nextInteger.add(BigInteger.valueOf(long));
}
for (int index = 0; index < integerText.length(); index += Character.charCount(integerText.codePointAt(index))) {
    Long digit = Long.parseLong(integerText.substring(index, index + Character.charCount(integerText.codePointAt(index)))));
    nextInteger = nextInteger.add(BigInteger.valueOf(long));
}
for (int index = 0; index < integerText.length(); index++) {
    long digit = Long.parseLong(integerText.substring(index, index + 1)));
    nextInteger = nextInteger.add(BigInteger.valueOf(digit));
}
for (int index = 0; index < integerText.length(); index += Character.charCount(integerText.codePointAt(index))) {
    Long digit = Long.parseLong(integerText.substring(index, index + Character.charCount(integerText.codePointAt(index)))));
    nextInteger = nextInteger.add(BigInteger.valueOf(digit));
}
added 14 characters in body
Source Link
Bobby
  • 8.2k
  • 2
  • 35
  • 43

That said, depending on the size of the number, toString might consume a lot of memory. The alternative would be to calculate the digits (through divideAndRemainder), but that will create additional Object instances during calculation. So, it depends on your use-case, I guess. Right now the implementation (when handed a "malformed" value) could be used to run a DoS by using up a lot of memory or aborting the operation with a stack overflow. Using divideAndRemainder and a loop instead of recursion would mean more GC pressure to fix thisbut would remedy these issues.


```java
for (int index = 0; index < integerText.length(); index += Character.charCount(integerText.codePointAt(index))) {
    Long digit = Long.parseLong(integerText.substring(index, index + Character.charCount(integerText.codePointAt(index)))));
    nextInteger = nextInteger.add(BigInteger.valueOf(long));
}

That said, depending on the size of the number, toString might consume a lot of memory. The alternative would be to calculate the digits (through divideAndRemainder), but that will create additional Object instances during calculation. So, it depends on your use-case, I guess. Right now the implementation (when handed a "malformed" value) could be used to run a DoS by using up a lot memory or aborting the operation with a stack overflow. Using divideAndRemainder and a loop instead of recursion would mean more GC pressure to fix this issues.


```java
for (int index = 0; index < integerText.length(); index += Character.charCount(integerText.codePointAt(index))) {
    Long digit = Long.parseLong(integerText.substring(index, index + Character.charCount(integerText.codePointAt(index)))));
    nextInteger = nextInteger.add(BigInteger.valueOf(long));
}

That said, depending on the size of the number, toString might consume a lot of memory. The alternative would be to calculate the digits (through divideAndRemainder), but that will create additional Object instances during calculation. So, it depends on your use-case, I guess. Right now the implementation (when handed a "malformed" value) could be used to run a DoS by using up a lot of memory or aborting the operation with a stack overflow. Using divideAndRemainder and a loop instead of recursion would mean more GC pressure but would remedy these issues.

for (int index = 0; index < integerText.length(); index += Character.charCount(integerText.codePointAt(index))) {
    Long digit = Long.parseLong(integerText.substring(index, index + Character.charCount(integerText.codePointAt(index)))));
    nextInteger = nextInteger.add(BigInteger.valueOf(long));
}
Source Link
Bobby
  • 8.2k
  • 2
  • 35
  • 43

Does it have to be recursive?

If I see this right, this could be wrapped in a loop just as easy. Theoretically a BigInteger is only limited by the amount of memory you have, so theoretically this method could be made to overflow the stack.

BigInteger must support values in the range -2^Integer.MAX_VALUE (exclusive) to +2^Integer.MAX_VALUE (exclusive) and may support values outside of that range. An ArithmeticException is thrown when a BigInteger constructor or method would generate a value outside of the supported range. The range of probable prime values is limited and may be less than the full supported positive range of BigInteger. The range must be at least 1 to 2^500000000.

From the BigInteger Javadoc.

That said, depending on the size of the number, toString might consume a lot of memory. The alternative would be to calculate the digits (through divideAndRemainder), but that will create additional Object instances during calculation. So, it depends on your use-case, I guess. Right now the implementation (when handed a "malformed" value) could be used to run a DoS by using up a lot memory or aborting the operation with a stack overflow. Using divideAndRemainder and a loop instead of recursion would mean more GC pressure to fix this issues.


public final class SimpleBigIntegerChecksum {
    
    private SimpleBigIntegerChecksum() {}

Very nice. I see rarely helper/utility classes which are "correctly" defined, so this is nice.


    public static int hash(final BigInteger bi) {

I'm not a fan of the name bi, bigInteger or value might be better fits.


        if (bi.compareTo(BigInteger.ZERO) < 0) {
            // Omit the leading minus sign:
            return hash(bi.negate());
        }

This will create a new BigInteger instance and recurse one level. Maybe skipping the minus in the String representation would be a cheaper option?


       final String integerText = bi.toString();

I'm not that happy with that name, maybe valueAsString or bigIntegerString would be better ones?


        if (integerText.length() == 1) {
            // Once the hash is only one character long, we are done:
            return Integer.parseInt(integerText);
        }

That check could be moved above by testing whether bi is less than 10. That would also mean you don't need to do BigInteger -> String -> int conversion but BigInteger -> int.


        // Sum up all the digits in the current big integer:
        for (final char ch : integerText.toCharArray()) {
            nextInteger = nextInteger.add(BigInteger.valueOf((long)(ch - '0')));
        }

You could also use substring, for example:

for (int index = 0; index < integerText.length(); index++) {
    long digit = Long.parseLong(integerText.substring(index, index + 1)));
    nextInteger = nextInteger.add(BigInteger.valueOf(long));
}

That has the upside that it does not rely on the ASCII/UTF-8-Page (and knowledge of it), but has the downside of being more verbose. One could use the BigInteger constructor to create the BigInteger directly from the String, skipping the parse to long, but that would also skip the BigInteger cache which is used by valueOf.

Note that the (correct) code-point aware version would look like this:


```java
for (int index = 0; index < integerText.length(); index += Character.charCount(integerText.codePointAt(index))) {
    Long digit = Long.parseLong(integerText.substring(index, index + Character.charCount(integerText.codePointAt(index)))));
    nextInteger = nextInteger.add(BigInteger.valueOf(long));
}

That said, splitting the loop into two lines would add greatly readability:

        for (final char ch : integerText.toCharArray()) {
            long digit = (long)(ch - '0');
            nextInteger = nextInteger.add(BigInteger.valueOf(digit));
        }

Also, ch isn't that great of a name either.


        // Eighteen nines = 90 + 8 * 9 = 90 + 72 = 162, checksum must be 9:
        System.out.println(hash(BigInteger.valueOf(999999999999999999L)));

I know this is only an example, but this comment/code combination should either be a proper test, or the comment should also be printed to stdout.