2
\$\begingroup\$

I am relatively new to coding and could use some help figuring out how I could improve my coding technique and habits. Here's an example of a working Java program I wrote that converts a number of some radix (from 2 to 36) to another radix (from 2 to 36).

Any tips or criticisms?

class RadixConverter {
    // command line arguments: (number) (radix of number) (radix to be converted to)
    public static void main(String[] args) {
        String n = args[0];                                   // n kept as String to support alphabetic representations of numerals for larger radixes
        int startRadix = Integer.parseInt(args[1]);
        int endRadix = Integer.parseInt(args[2]);
        int decimal = convertToDecimal(n, startRadix);
        String result = convertToBase(decimal, endRadix);
        System.out.printf("%s (%d) = %s (%d)%n", n, startRadix, result, endRadix);
    }

    public static int convertToDecimal(String n, int startRadix) {
        int sum = 0;
        for (int i = 0; i < n.length(); i++) {                                                 // begins at first digit and ends at last digit
            char digit = n.charAt(i);
            int placeValue = (int) Math.pow(startRadix, n.length() - i - 1);                   // place value of digit's column
            if ((digit >= 48) && (digit <= 57) && (digit - 48 < startRadix)) {                 // if digit is numerical and is within specified radix
                sum += (digit - 48) * placeValue;
            } else if ((digit >= 65) && (digit <= 90) && (digit - 65 + 10 < startRadix)) {     // if digit is alphabetic and within specified radix
                sum += (digit - 55) * placeValue;
            } else {
                throw new IllegalArgumentException("\n\nThe input number is invalid or contains digits outside of its specified radix.\n");
            }
        }
        return sum;
    }

    public static String convertToBase(int decimal, int endRadix) {
        StringBuilder sb = new StringBuilder("");
        do {
            int rem = decimal % endRadix;
            if ((rem <= 9) && (endRadix <= 36) && (endRadix >= 2)) {
                sb.append(Integer.toString(rem));
            } else if ((rem <= 35) && (endRadix <= 36) && (endRadix >= 2)) {
                sb.append((char) (rem + 55));
            } else {
                throw new IllegalArgumentException("\n\nThe output radix must be from 2 to 36.\n");
            }
            decimal /= endRadix;
        } while (decimal != 0);
        return reverseOrder(sb.toString());
    }

    public static String reverseOrder(String s) {
        StringBuilder sb = new StringBuilder("");
        for (int i = s.length() - 1; i >= 0; i--) {    // executes each character in reverse order
            sb.append(s.charAt(i));
        }
        return sb.toString();
    }
}
\$\endgroup\$
1
\$\begingroup\$

You're re-inventing the wheel. In the Integer class, there are methods for converting numbers to and from Strings with a radix parameter. So the following class does the job:

package test;

public class Test {

    public static void main(String[] args) {
        int inRadix = Integer.parseInt(args[1]);
        int outRadix = Integer.parseInt(args[2]);
        int value = Integer.parseInt(args[0], inRadix);
        String result = Integer.toString(value, outRadix);
        System.out.printf("%s (%d) = %s (%d)%n", value, inRadix, result, outRadix);
    }
}

But let's for a moment suppose the methods weren't there and look at your solution.

First, the method name convertToDecimal shows a (quite common, I have to say) mis-understanding. An int is internally represented in binary (radix 2), not decimal (radix 10). So convertToInt would be a better name.

I'd extract the character-versus-digit-value logic into methods int digitValueOf(char ch) and char digitCharFor(int digit) (By the way, the Character class has these methods).

I'd replace the numbers like 48 by the characters they represent (e.g. '0'). Not every reader knows the Unicode table by heart.

I'd replace the Math.pow() by consecutive integer multiplication. Turn the loop the other way round, starting from the last digit with placeValue = 1. In every iteration, multiply placeValue by radix. Math.pow() is slower and less exact than integer arithmetic.

I'd get into the habit of adding Javadoc even to trivial methods. After a year or so, you'll be happy.

Personally, I'm lazy with comments inside methods. A good method name, a Javadoc explaining what the method does (not how), and a straightforward implementation within the method are typically enough for a reader to understand what's going on.

Good is the usage of StringBuilder instead of concatenating Strings (quite common performance killer).

Good is throwing an exception in case of illegal characters.

Good is following the standard Java naming conventions.

\$\endgroup\$
  • \$\begingroup\$ Thank you for the great feedback. I noticed the first line of the code you provided has "package test;". Perhaps this is a silly question, but what is that for? What does it do? Additionally, regarding the convertToDecimal method, I'm still not quite sure I understand. It is my understanding that all primitive datatypes are binary, so why is it fallacious to refer to something that represents decimal numbers as "decimal"? Thanks again. \$\endgroup\$ – Simeon Rediger Aug 14 '17 at 8:04
  • \$\begingroup\$ package test places your class into a namespace. So the full-length name of my class is test.Test. Placing code into a package makes it less likely to collide with some library's other class. An established convention is to use a reversed internet domain as prefix for your packages, e.g. com.stackexchange.codereview.test.whatever - if we were the "owners" of stackexchange. Then there can't be any collisions. \$\endgroup\$ – Ralf Kleberhoff Aug 14 '17 at 14:02
  • \$\begingroup\$ Regarding "decimal": your convertToDecimal() method returns an int, which is, as you correctly say, a binary thing, and has nothing to do with a decimal representation - decimal is the duty of toString() methods and so on. \$\endgroup\$ – Ralf Kleberhoff Aug 14 '17 at 14:04
  • \$\begingroup\$ Regarding package test, I suppose I'll be able to better understand that later once I've acquired more knowledge about programming in general. Thanks. \$\endgroup\$ – Simeon Rediger Aug 14 '17 at 21:09
  • 1
    \$\begingroup\$ Regarding convertToDecimal(), I still can't quite wrap my brain around what you're saying. All primitive datatypes are binary, but all calculations are, for all intents and purposes it seems, decimal. For instance, the statement System.out.print(1 + 1); prints "2"—not "10" as binary would have it. So when a—say—hexadecimal number "B3" is transformed by the method into the decimal number "179", why is it fallacious to call that a decimal number if it works, acts, and looks like a decimal number even though the computer knows it to be 10110011? \$\endgroup\$ – Simeon Rediger Aug 14 '17 at 21:16

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.