5
\$\begingroup\$

Any feedback is welcome:

public class HexToDecimal {

    static String hexRep = "0123456789ABCDEF";

    public static void main(String[] args) {
        System.out.println(HexToDecimal.hexToDecimal("AB"));
    }

    public static int hexToDecimal(String hex) {

        int counter = hex.length()-1;
        int sum = 0;

        for (char c:hex.toCharArray()) {
            int i = hexRep.indexOf(c);
            sum = (int) (sum + (Math.pow(16,counter))*i);
            counter--;
        }

        return sum;
    }

}
\$\endgroup\$
  • 1
    \$\begingroup\$ I presume this is an exercise or homework, because there are built-in methods to do this. \$\endgroup\$ – Bart Kiers Jan 30 '15 at 10:34
  • 6
    \$\begingroup\$ @BartKiers I am too old for homeworks. \$\endgroup\$ – Koray Tugay Jan 30 '15 at 10:38
  • 4
    \$\begingroup\$ I don't know. I mean, lots of adults/working people do some courses in the evening. It could be homework and the teacher said you couldn't use Integer.valueOf("AB", 16) :) \$\endgroup\$ – Bart Kiers Jan 30 '15 at 10:47
  • 2
    \$\begingroup\$ The code doesn't fulfil its specifications because it doesn't return a decimal number, it returns an int, which is a binary representation of the number. People often confuse this but it's nevertheless wrong. To return a decimal representation you'd need to either return an array of decimal digits or a string of decimal digits, just like your input is a string of hexadecimal digits. \$\endgroup\$ – Konrad Rudolph Jan 31 '15 at 12:36
  • 1
    \$\begingroup\$ @KorayTugay The difference is that you store the decimal digits indidually. At the moment you are simply not returning a decimal number, full stop. When you return an array or a string of decimal digits it doesn’t matter how the digits themselves are stored, what matters (for the definition of a decimal number) is that the number consists of decimal digits. \$\endgroup\$ – Konrad Rudolph Jan 31 '15 at 15:21
15
\$\begingroup\$

Java's Integer class has a built-in method to parse a hex number (string) into an int:

Integer.valueOf("AB", 16)

W.R.T. your code, here are some observations:

  • hexRep should be final
  • Ideally your method should also accept lower-case text (just call toUpperCase() on the string)
  • Include some unit tests perhaps?
  • I wouldn't put that single method in a class of its own. More appropriate would be to put it in a more general utility class
\$\endgroup\$
9
\$\begingroup\$
static String hexRep = "0123456789ABCDEF";

Imagine if I put some code in the same package and did HexToDecimal.hexRep = ""; Now I broke your program!

This one needs to be private static final


Your hexToDecimal method is not checking for invalid input. Calling hexToDecimal("7G") causes incorrect results, without any kind of warning or indication that an error occurred. Throwing an exception is the least you can do! An exception should probably also be thrown if it goes outside the valid int range.

Also, the string should be converted to uppercase inside the method to also support strings like "7a8d".

\$\endgroup\$
7
\$\begingroup\$

To get a normal integer from a hex string you can use the built-in Integer.parseInt(String, int) which simplifies things to Integer.parseInt("AB", 16);

The actual parsing code can be simplified by knowing that "AB"*16 == "AB0" this allows you to do away with the pow operation:

for (char c:hex.toCharArray()) {
    int i = hexRep.indexOf(c);
    sum = sum*16 + i;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ sum * 16 can be replaced with sum << 4 \$\endgroup\$ – Pimgd Jan 30 '15 at 10:47
  • 4
    \$\begingroup\$ @Pimgd that tends to be overkill and unreadable for new programmers, java will do that for you soon enough. \$\endgroup\$ – ratchet freak Jan 30 '15 at 10:51
5
\$\begingroup\$

Naming

Strictly speaking, your method does not convert the hex string into decimal. Rather, it parses the hex string as an integer. It's PrintWriter.println(int) that actually converts it to base-10 notation. Until then, it's just a signed 32-bit number, not in any particular base. (Well, the Java Language Specification doesn't say so, but you know that your computer works in base 2.)

Therefore, if you wanted to reinvent the wheel, you should rename your function to

public static int fromHex(String hex)

Bugs

You don't do any validation, so any unexpected characters would be interpreted as a -1 hexdigit, since that's how String.indexOf() behaves. There are a number of reasonable behaviours in the face of unexpected input, but this is one of the least expected failure modes. (Throwing an exception would be best.) Note that lowercase a-f are unsupported, and thus fail in that manner.

\$\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.