2
\$\begingroup\$

Nothing more to add than the title. Looking for code review, optimizations and best practices.

public final class BaseAdder {

    private BaseAdder() { }

    /**
     * Given two numbers and their bases, adds the number
     * and returns the result in the same base.
     * 
     * This code supports max-base 26.
     * 
     * 
     * @param num1  the first number to be added
     * @param num2  the second number to be added
     * @param base  the base of the numbers
     */
    public static String add (String num1, String num2, int base) {

        if (base <  2) throw new IllegalArgumentException("The base should at least be 2. Input base is: " + base);
        if (base > 26) throw new IllegalArgumentException("The base should be less than 26. Input base is: " + base);

        /*
         * http://stackoverflow.com/questions/355089/stringbuilder-and-stringbuffer-in-java
         * and StringBuilder is intended as a drop in replacement for StringBuffer where synchronisation is not required –  Joel Dec 23 '09 at 8:52
         * 
         */
        final StringBuilder stringBuilder = new StringBuilder();
        int carry = 0;
        for (int i = num1.length() - 1; i >= 0; i--) {
            int x =  getIntValue(num1.charAt(i), base) +   getIntValue(num2.charAt(i), base) + carry;

            if (x >= base) { 
                carry = 1;
                x = x - base;
            } else {
                carry = 0;
            }
            stringBuilder.append(getCharValue(x, base));
        }

        if (carry == 1) stringBuilder.append(1);

        return stringBuilder.reverse().toString();
    }


    public static int getIntValue(char ch, int base) {
        if (ch > getCharValue(base - 1, base)) {
            throw new IllegalArgumentException(" invalid character " + ch + " for input base " + base);
        }

        if (ch >= '0' &&  ch <= '9') {
            return ch - '0';
        } 
        Character.toUpperCase(ch);
        return ch - 'A' + 10;
    }


    public static char getCharValue(int x, int base) {
        if (x >= base) {
            throw new IllegalArgumentException(" invalid number " + x + " for input base " + base);
        }

        if (x >= 0 &&  x <= 9) {
            return (char) (x + '0');
        } 

        return (char)(x + 'A' - 10);
    }

public static void main(String[] args) {  
    assertEquals("100", BaseAdder.add("10", "10", 2));
    assertEquals("11", BaseAdder.add("10", "01", 2));

    assertEquals("25", BaseAdder.add("10", "15", 10));

    assertEquals("4A", BaseAdder.add("1C", "2D", 15));
    assertEquals("1B9", BaseAdder.add("CC", "ED", 16));

    assertEquals("1B", BaseAdder.add("C", "D", 14));
    assertEquals("1A", BaseAdder.add("C", "D", 15));
    assertEquals("19", BaseAdder.add("C", "D", 16));
}
} 
\$\endgroup\$
  • \$\begingroup\$ You have an extra } at the very end (assuming everything already matches up). \$\endgroup\$ – Jamal Jun 9 '14 at 1:59
2
\$\begingroup\$

Your solution is incorrect by inspection, due to its disregard for num2.length().

The simplest solution would be to use BigInteger. Assuming that you are deliberately , you don't need to reinvent the wheel from scratch. Take advantage of Character.digit(int codePoint, int radix) and Character.forDigit(int digit, int radix).

A more flexible design would be an Accumulator object, which would allow you to add several numbers more efficiently.

public class Accumulator {
    public Accumulator(int radix) { … }
    public void add(String num) { … }
    public String toString() { … }
}
\$\endgroup\$
4
\$\begingroup\$

The limit of the maximum radix of 26 sounds odd, why not use the full alphabet of characters up to 10 digits, and 26 letters, giving a total of radix-36?

You have not included a reinventing-the-wheel tag, and thus I can suggest the following:

public static String add (String num1, String num2, int base) {
    if (base > 26) {
       // arbitrary limitation to 26... BigInt supports to 36.
    }
    BigInteger bione = new BigInteger(num1, base);
    BigInteger bitwo = new BigInteger(num2, base);
    return bione.add(bitwo).toString(base);
}

I believe that satisfies the review, optimization and best-practices request.

\$\endgroup\$
3
\$\begingroup\$

Looks pretty good to me. I just have a few non-functional comments to add:

  • The add JavaDoc is incomplete, as you state the upper-limit of base but not the lower-limit. Surely someone might wonder what the sum of 01 and 001 is.

  • Maybe a transcription error, but there's some strange white spacing in places, which jumps out at me. For example,

    • int x = getIntValue(num1.charAt(i), base) + getIntValue(num2.charAt(i), base) + carry;

    • throw new IllegalArgumentException(" invalid number " + x + " for input base " + base);

  • And you have several unbracketted if statements, which is a bad practice to get into.

\$\endgroup\$
1
\$\begingroup\$
  1. Why do you need to put the final in front of the StringBuilder.
  2. Why didn't you assign the initial buffer size to the string builder
  3. Why didn't you cache the value used for checking if a single character is valid or not.
  4. You should document that you expect the 2 input strings to be of equal length. (I know you expect that because I see your test case where you have the "01")

It's funny since you are doing something so counter-intuitive, there are weird ways of finding what's wrong with it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.