Binary Converting Code

Question

I made a program that converts a decimal number given from the user, and converts it to a binary number. For example, if I entered 2, it would return 10. It then, counts the amount of ones and zeros in the binary conversion. Please tell me any way my code can be either, more efficient, or if there is an error with my code. If there are any bad practices used, please tell me. Indentation is also important to me.

package Conversions;
import java.util.Scanner;

public class BinaryChallenge {
    @SuppressWarnings("resource")
    public static void main(String[] args) {
    int number; 

    Scanner scan = new Scanner(System.in);
    int oneCount = 0;
    int zeroCount = 0;
    System.out.println("Enter a positive integer");
    number = scan.nextInt();
    String binary = "";
    if (number < 0) {
        System.out.println("Error: Not a positive integer");
    } else { 
        while(number > 0) {
            int mod = (number % 2);
            number /= 2;
            binary = Integer.toString(mod) + binary;
        }
        System.out.println("The binary representation of your integer is: " + binary + ".");
    }

    for (int i = 0; i < binary.length(); i++) {
        if (binary.charAt(i) == '1') {
            oneCount++;
        } else if(binary.charAt(i) == '0') {
            zeroCount++;
        }
    }
    System.out.println("The binary representation of your integer has: " + oneCount + " ones.");
    System.out.println("The binary representation of your integer has: " + zeroCount + " zeros.");
    System.out.println("The binary representation of your integer has 0 twos... duh.");
}   
}

Answer 1

Input validation?

At first glance, this looks like some sort of input validation:

if (number < 0) {
    System.out.println("Error: Not a positive integer");
} else {
    // ...
}

// ...
System.out.println("The binary representation of your integer has: " + oneCount + " ones.");
System.out.println("The binary representation of your integer has: " + zeroCount + " zeros.");

But it's not... If the input is negative, processing will happily continue and print that the number has 0 zeros and 0 ones, which is not true.

Lastly, errors should be printed on System.err instead of System.out.

Variable scope

Declare variables where you really need them, not sooner. number, zeroCount, oneCount can all be declared later.

Decompose to smaller units

Instead of dumping the code in a single main method, it would be better to decompose the multiple small methods, each responsible for one thing. For example, you could have these methods:

• read an int from standard input
• convert an int to a binary string
• count the number of ones in a binary string
  • the number of zeros is length minus the number of ones

Simplify

Instead of this:

binary = Integer.toString(mod) + binary;

You could simplify as:

binary = mod + binary;

Reinventing the wheel

Integer.toBinaryString(...) does the same thing as your main loop.

Answer 2

Algorithm

The other answers are right to say that you should use Integer.toBinaryString(...), but if you have to implement the binary conversion yourself you'd be better off using bitwise operators. Those would allow you to support negative numbers as well as positive. You could also save time and lines by counting 1s and 0s as you add them to the result string.

Practice

You're using a @SuppressWarnings("resource") annotation instead of closing your scanner. You should close it or, if you're using Java 7, use a try-with-resources statement instead. This will make sure your scanner gets closed.

Code

You use 3 different int variables which you create at different points in your code. It'd be better to put them in the same definition, or at least near each other. You can get better performance out of a StringBuilder object rather than concatenating to the same string. It's not really an issue for such simple code, but it might be a problem in the future.

Answer 3

• If you really want to stick to strings you could use Integer.toBinaryString(...):

  Returns a string representation of the integer argument as an unsigned integer in base 2.

• But you also could use BitSet with its valueOf(long[] longs), cardinality() and length() methods instead of String binary to get oneCount and zeroCount. It also handles negative numbers implicitely.

• If you use System.out a lot you could use:

  import static java.lang.System.out;
  

  and then just:

  out.print[f|ln](...);
  

• Instead of println() and string concatenation within I'd use printf(...), e.g.:

  out.printf("The binary representation of your integer has %d ones.", oneCount);
  

• I'd use:

  out.print("Enter a positive integer: ");
  

  (for input on the same line) but that might be personal taste.

• Limiting to positive numbers is, well, a limitation. Mathematically spoken, also negative numbers can be represented by a binary number.