patternjavaModerate
Binary Converting Code
Viewed 0 times
codeconvertingbinary
Problem
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) {
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.");
}
}Solution
Input validation?
At first glance, this looks like some sort of input validation:
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
Variable scope
Declare variables where you really need them, not sooner.
Decompose to smaller units
Instead of dumping the code in a single
it would be better to decompose the multiple small methods,
each responsible for one thing.
For example, you could have these methods:
Simplify
Instead of this:
You could simplify as:
Reinventing the wheel
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
intfrom standard input
- convert an
intto 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.Code Snippets
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.");binary = Integer.toString(mod) + binary;binary = mod + binary;Context
StackExchange Code Review Q#110127, answer score: 10
Revisions (0)
No revisions yet.