3
\$\begingroup\$

I know I could achieve this using System.out.println(Integer.toBinaryString(num)); in Java. However, I wanted to do it this way.

private static void toBinary(int x) {

    Scanner in = new Scanner(System.in);
    int num = in.nextInt();
    String s = "";

    while (num >= 2) {

      s = s + valueOf(num % 2);

      num = (int) num / 2;
    }

    String newS = new StringBuilder(s).reverse().toString();

    System.out.print(num + newS);
  }
\$\endgroup\$
3
  • 2
    \$\begingroup\$ What is your argument x used for? \$\endgroup\$
    – JS1
    Mar 7, 2017 at 17:31
  • \$\begingroup\$ What is the question? \$\endgroup\$
    – Myridium
    Mar 7, 2017 at 23:01
  • \$\begingroup\$ @Myridium you are on Codereview \$\endgroup\$
    – Egek92
    Mar 8, 2017 at 7:57

1 Answer 1

8
\$\begingroup\$

Functional extraction

Your method does not do toBinary, it does:

  • read input from user
  • convert to binary
  • print back for user

When you have methods that do multiple things it becomes hard to isolate and reuse the logic. Your code should be split in to parts, and the toBinary method should take the int input and return a String

Complicated end-of-loop handling

The while condition while (num >= 2) { is a complicated construct, that needs a comment. That logic is key to the process, and guarantees that you are left with a "dangling" 1 for the high-bit, and also handles the 0 input case. It took me a number of minutes to figure out how that code works, and that's not useful for your readers... (or yourself, a year in the future).

As it happens, a do-while loop is the structure you want, and it simplifies the logic a whole bunch....

valueOf(...) mystery

I assume you have done a static import of the String class? Otherwise the valueOf method is not defined. Doesn't this defeat some of your "old-school" desire? Also, why not just use a simple 2-value lookup table. The digits are either 0 or 1, so it's simply:

char[] digits = new char[]{'0', '1'};

and then your valueOf can be replaced with:

digits[num % 2]

implicit type conversion

You do an implicit type conversion in the output num + newS which is not "old-school" either. Adding a number to a string is... complicated.

Unnecessary reversals

Why do the reverse mechanism? Why not just build the digit string up backwards to start with? Instead of:

s = s + valueOf(num % 2);

have:

s = valueOf(num % 2) + s;

String concatenation

String concatenation with the + operator is often frowned on... in this instance, it makes the code cleaner, and I am leaving it in. The alternative is to use a StringBuilder, but that has other issues in this use-case. Like you, I would use string concatenation...

Conclusion

All told, I would recommend something like:

private static final char[] digits = "01".toCharArray();

private static String toBinaryRL(int num) {
    if (num < 0) {
        throw new IllegalArgumentException("toBinary can only handle positive integers, not " + num);
    }
    String binary = "";
    do {
        binary = digits[num % 2] + binary;
        num /= 2;
    } while (num > 0);
    return binary;
}

Further, this process can be easily extended in to any "base", by extending the digits, and adding a parameter to the function:

private static final char[] digits = "0123456789abcdefghijklmnopqrstuvwxyz".toCharArray();

private static String toBase(int num, int base) {
    if (num < 0) {
        throw new IllegalArgumentException("toBase can only handle positive integers, not " + num);
    }
    if (base <= 1 || base >= digits.length) {
        throw new IllegalArgumentException("Cannot convert to base " + base);
    }
    String result = "";
    do {
        result = digits[num % base] + result;
        num /= base;
    } while (num > 0);
    return result;
}

private static String toBinaryB(int num) {
    return toBase(num, 2);
}
\$\endgroup\$
1
  • \$\begingroup\$ Thank you so much! another thing I've realized; my code doesn't work with negative numbers. \$\endgroup\$
    – Egek92
    Mar 7, 2017 at 16:36

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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