patternjavaMinor

Base Changing in Java, improved version

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

improvedversionjavachangingbase

Problem

This is a follow up to my previous question, Base-changing numbers in Java

I have followed @Pimgd's advice in the answer to that question, and I'm posting the improved code for any further review.

Note:

I didn't bother with whitespace in the test method, because I don't use it anyway. It also isn't up for review, it's just there to help people to run the program quick and dirty.

Here's the code:

``


/**
 * Converts a number in digit representation from one base to another
 * Not guaranteed to work with bases above 65,451 (more characters than which the lookup can use for substituting digits)
 *
 * @author Tamoghna Chowdhury
 * @version 1.2
 */
public class BaseConverter {
    protected static final String negativeBaseErrorMessage = "Negative or zero base values are illegal - supplied bases were %d & %d.", 
    invalidInputNumberErrorMessage = "The supplied number (%s) for base conversion is invalid.", 
    tooLargeBaseForLookupErrorMessage = "Not enough available characters for substitution. Number of available characters is %d , minimum required number is %d";

    private static String lookup = "0123456789ABCDEFGHIJKLMNOPQRSTWXYZabcdefghijklmnopqrstwxyz+/=,?!;:\"'^

~|\\@#$%&*_<>(){}";

private static void updateLookup(int base) {
int charsToAdd = base - lookup.length();
if (charsToAdd > 0) {
for (int i = 0; i 0; ++i) {
if ((!lookup.contains("" + (char) i)) &&
(!Character.isISOControl((char) i)) &&
(!Character.isWhitespace((char) i)) &&
((char) i != '.') && (char) i != '-') {
lookup += (char) i;
--charsToAdd;
}
}
}
if (charsToAdd > 0) {
throw new IllegalArgumentException(String.format(tooLargeBaseForLookupErrorMessage, lookup.length(), base));
}
}
public static long convertToNumber(String inputNumber, int from_base) {
if (f

Solution

private static void updateLookup(int base) {
    int charsToAdd = base - lookup.length();
    if (charsToAdd > 0) {
        for (int i = 0; i  0; ++i) {
            if ((!lookup.contains("" + (char) i)) &&
                    (!Character.isISOControl((char) i)) &&
                    (!Character.isWhitespace((char) i)) &&
                    ((char) i != '.') && (char) i != '-') {
                lookup += (char) i;
                --charsToAdd;
            }
        }
    }
    if (charsToAdd > 0) {
        throw new IllegalArgumentException(String.format(tooLargeBaseForLookupErrorMessage, lookup.length(), base));
    }
}

So thing is, this method is still too slow for really huge bases. On one hand, you could say "such a huge base is overkill". On the other hand, denial of service attacks are a thing, and servers are better at recovering from crashes than infinite loops.

But the thing is, you don't add other characters to lookup, nor do you ever clear lookup.

So you don't need to start from 0 whenever you enter the loop. You can start from where you left off!

So keep track of a lastUsedCharacterIndex of sorts (might need a better name).

Second, if we assume that the character set doesn't contain duplicates, then any newly added character cannot be encountered via incrementing i. So the only thing you need to test against is the value of lookup before you entered the loop.

Once we make these changes, you'd think the issue is fixed, but no! String concatenation in a loop KILLS performance, and any lint tool of quality will give you a warning for += to a string in a loop. So you will have to use something like StringBuilder. By using a StringBuilder we also stored the old value in lookup, so there's no need for a seperate variable (like I suggested earlier).

Once we do that...

private static StringBuilder builder = new StringBuilder(lookup);

private static void updateLookup(int base) {
    int charsToAdd = base - lookup.length();
    if (charsToAdd > 0) {
        for (int i = 0; i  0; ++i) {
            if ((!lookup.contains("" + (char) i)) &&
                    (!Character.isISOControl((char) i)) &&
                    (!Character.isWhitespace((char) i)) &&
                    ((char) i != '.') && (char) i != '-') {
                builder.append((char) i);
                --charsToAdd;
            }
        }
    }
    lookup = builder.toString();
    if (charsToAdd > 0) {
        throw new IllegalArgumentException(String.format(tooLargeBaseForLookupErrorMessage, lookup.length(), base));
    }
}

By just applying StringBuilder, what used to take 5 seconds (and then broke, on ideone, after hitting 40000 characters), now runs in 0.14 seconds.

Another performance improvement would be to add && charsToAdd < (Character.MAX_VALUE - lookup.length()) - check if it's going to fit BEFORE you fill up the lookup table.

if (from_base <= 0 || to_base <= 0) {
        //negative or zero bases can't be handled using simple integer arithmetic
        throw new IllegalArgumentException(String.format(negativeBaseErrorMessage, from_base, to_base));
    }

This guard clause should placed before the isNegative check because it doesn't rely on the negative check.

Code Snippets

private static void updateLookup(int base) {
    int charsToAdd = base - lookup.length();
    if (charsToAdd > 0) {
        for (int i = 0; i < Character.MAX_VALUE && charsToAdd > 0; ++i) {
            if ((!lookup.contains("" + (char) i)) &&
                    (!Character.isISOControl((char) i)) &&
                    (!Character.isWhitespace((char) i)) &&
                    ((char) i != '.') && (char) i != '-') {
                lookup += (char) i;
                --charsToAdd;
            }
        }
    }
    if (charsToAdd > 0) {
        throw new IllegalArgumentException(String.format(tooLargeBaseForLookupErrorMessage, lookup.length(), base));
    }
}

private static StringBuilder builder = new StringBuilder(lookup);

private static void updateLookup(int base) {
    int charsToAdd = base - lookup.length();
    if (charsToAdd > 0) {
        for (int i = 0; i < Character.MAX_VALUE && charsToAdd > 0; ++i) {
            if ((!lookup.contains("" + (char) i)) &&
                    (!Character.isISOControl((char) i)) &&
                    (!Character.isWhitespace((char) i)) &&
                    ((char) i != '.') && (char) i != '-') {
                builder.append((char) i);
                --charsToAdd;
            }
        }
    }
    lookup = builder.toString();
    if (charsToAdd > 0) {
        throw new IllegalArgumentException(String.format(tooLargeBaseForLookupErrorMessage, lookup.length(), base));
    }
}

if (from_base <= 0 || to_base <= 0) {
        //negative or zero bases can't be handled using simple integer arithmetic
        throw new IllegalArgumentException(String.format(negativeBaseErrorMessage, from_base, to_base));
    }

Context

StackExchange Code Review Q#123116, answer score: 3

Revisions (0)

No revisions yet.