7
\$\begingroup\$

I've written an algorithm which can multiply two hex values and return a hex as a result. What do you think about the below solution?

package com.datastructute.arraystring;

public class hexadecimal {
    public static void main(String[] args) {

        System.out.println(multiplyHex("AE08FE2111111111", "BF"));
        System.out.println(multiplyHex("DF", "BC"));        // A3C4
        System.out.println(multiplyHex("ADF398", "BA48"));  // 7e93e8f2c0

        // Test Screnarios
        System.out.println(multiplyHex(null, null));
        System.out.println(multiplyHex("  ", "  "));
        System.out.println(multiplyHex("hyh", "hyhy"));
        System.out.println(multiplyHex("abyh", "ashyhy"));
        System.out.println(multiplyHex("-1-1-1", "-1-1-1"));
        System.out.println(multiplyHex("AE08FE2111111111AE08FE2AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE2111111111AE08FE21111111111111AE08FE2111111111AE08FE21111111111111AE08FE21111AE08FE211111111111111", "AE0AE08FE21111111118FE2111111111AE08FE2111111111")); 
    }

    public static String multiplyHex(String str1, String str2)  {
        if(str1 == null || str2 == null)
            return "Null values are not accepted.";

        char[] hex1 = str1.toCharArray();
        char[] hex2 = str2.toCharArray();

        int[][] ArrHexMatrix;
        int arrLength = hex1.length + hex2.length;
        int arrIndexLength = hex1.length + hex2.length - 1;
        int lines = hex2.length;
        ArrHexMatrix = new int[hex2.length][arrLength];

        int mod = 0;
        int carry = 0;
        int count = 0;
        int index = 0;

        for (int i = lines - 1; i >= 0; i--) {

            carry = 0;
            count = 0;
            for (int j = hex1.length - 1; j >= 0; j--) {
                try {
                    if(getInt(hex2[i])==-1 || getInt(hex1[j])==-1)
                        return "Wrong chracter";

                    mod = (getInt(hex2[i]) * getInt(hex1[j]) + carry) % 16;
                    carry = ((getInt(hex2[i]) * getInt(hex1[j])) + carry) / 16;

                    if (j == 0) {
                        ArrHexMatrix[index][arrIndexLength - count - index] = mod;
                        ArrHexMatrix[index][arrIndexLength - count - 1 - index] = carry;
                    } else {
                        ArrHexMatrix[index][arrIndexLength - count - index] = mod;
                    }

                } catch (Exception e) {
                    e.printStackTrace();
                }
                count++;
            }
            index++;
        }

        int sum = 0;
        mod = 0;
        carry = 0;
        count = 0;

        char[] total = new char[arrLength];
        for (int i = arrLength - 1; i >= 0; i--) {
            sum = 0;
            for (int j = 0; j < lines; j++) {
                sum += ArrHexMatrix[j][i];
            }
            mod = (sum + carry) % 16;
            carry = (sum + carry) / 16;

            try {
                total[i] = getChar(mod);
            } catch (Exception e) {
                // TODO Auto-generated catch block
                e.printStackTrace();
            }
        }

        return String.valueOf(total);
    }

    private static int getInt(char chr) {
        switch (chr) {
        case '0':
            return 0;
        case '1':
            return 1;
        case '2':
            return 2;
        case '3':
            return 3;
        case '4':
            return 4;
        case '5':
            return 5;
        case '6':
            return 6;
        case '7':
            return 7;
        case '8':
            return 8;
        case '9':
            return 9;
        case 'A':
            return 10;
        case 'B':
            return 11;
        case 'C':
            return 12;
        case 'D':
            return 13;
        case 'E':
            return 14;
        case 'F':
            return 15;
        default:
            return -1;
        }

    }

    private static char getChar(int val) throws Exception {
        switch (val) {
        case 0:
            return '0';
        case 1:
            return '1';
        case 2:
            return '2';
        case 3:
            return '3';
        case 4:
            return '4';
        case 5:
            return '5';
        case 6:
            return '6';
        case 7:
            return '7';
        case 8:
            return '8';
        case 9:
            return '9';
        case 10:
            return 'A';
        case 11:
            return 'B';
        case 12:
            return 'C';
        case 13:
            return 'D';
        case 14:
            return 'E';
        case 15:
            return 'F';
        default:
            throw new Exception();
        }
    }
}
\$\endgroup\$
12
\$\begingroup\$

Your getInt and getChar methods are very long and cumbersome, they can be replaced by this:

Using arrays:

private char[] values = new char[]{ '0', '1', '2', '3', '4', '5', '6', '7',
      '8', '9', 'A', 'B', 'C', 'D', 'E', 'F' };

char getInt(int i) {
    return values[i];
}

Java has already built-in methods to do this conversion though, so all that is needed is really:

int getInt(char chr) {
    return Character.digit(chr, 16);
}

char getChar(int val) {
    return Character.forDigit(val, 16);
}

Hexadecimal values are often showed in lower-case, personally I like that better because it makes it easier to separate the characters (a-f) from the digits (0-9). If you still want to use upper-case characters though, use this:

return Character.toUpperCase(Character.forDigit(val, 16));

    private static char getChar(int val) throws Exception {

Don't throw Exception, use IllegalArgumentException instead. IllegalArgumentException is a RuntimeException and therefore does not need to be caught (or declared in the throws declaration), but you are still free to do so if you would like. (Although I would not recommend it)


Learn to use JUnit to do the testing.

import static org.junit.Assert.*;

public class MultiplyHexTest {
    @Test
    public void testSomething() {
        assertEquals("A3C4", multiplyHex("DF", "BC"));
    }

    @Test(expected = IllegalArgumentException.class)
    public void testFailure() {
        multiplyHex("  ", "  ");
    }
}

ArrHexMatrix, being a name for a variable, should start with a lowercase character according to the Java naming conventions.

\$\endgroup\$
  • \$\begingroup\$ Thank you Simon, i ll look at junit testing. As you see the time complexity is O(n^2), do you know any better solution for this problem ? \$\endgroup\$ – talhakosen May 30 '14 at 14:09
  • \$\begingroup\$ @talhakosen The BigInteger class also has a multiply method with complexity O(n^2), so unfortunately I don't know any faster solution. \$\endgroup\$ – Simon Forsberg May 30 '14 at 17:39
  • 1
    \$\begingroup\$ +1 for suggesting JUnit. Reading the console output to verify results is time-consuming and error-prone. \$\endgroup\$ – Robert Snyder May 30 '14 at 18:02
  • 2
    \$\begingroup\$ @talhakosen I know there are sub O(n^2) multiplication algoritms... karatsuba comes to mind, introcs.cs.princeton.edu/java/78crypto/Karatsuba.java.html has a java implemenation to give an idea on the complexity etc. (wiki page talks about the O complexity \$\endgroup\$ – Foon May 31 '14 at 16:03
  • \$\begingroup\$ thanks @Foon, karatsuba has advantages when the length of input > 14. \$\endgroup\$ – talhakosen May 31 '14 at 21:44
10
\$\begingroup\$

Exceptions:

It is common practice to throw an IllegalArgumentException if the input values to a method are not legal. In the sepcific case of null values, there is debate about whether the right response is an IllegalArgumentException, or a NullPointerException. I prefer IllegalArgumentException....

Also, in Java, (and most languages), it is very easy to introduce bugs when making small changes to 1-liner conditionals (just ask apple). When making the changes correctly to 1-liners (without introducing bugs) the actual small changes require adding in braces, so small changes change a lot. In general, use braces, even for 1-liners.

The code:

    if(str1 == null || str2 == null)
        return "Null values are not accepted.";

Should be:

    if(str1 == null || str2 == null) {
        throw new IllegalArgumentException("Null values are not accepted.");
    }

One of the reasons for the exception, by the way, is... consider the following code:

System.out.println(multiplyHex("A", multiplyHex("B", null));

With your code in the current state, the result will be:

   "Wrong chracter"

Additionally, the "wrong character" message is .... useless. You shoudl have an exception that indicates what the wrong character is:

int hxval1 = getInt(hex1[j]);
if (hxval1 < 0) {
    throw new IllegalArgumentException(String.format("Illegal character '%s' at position %d in the input value: %s", hex1[j], j, str1));
}

Use exceptions

\$\endgroup\$
  • \$\begingroup\$ Thanks rolfl, i got it. As you see the time complexity is O(n^2), do you know any better solution for this problem ? \$\endgroup\$ – talhakosen May 30 '14 at 14:10

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.