2
\$\begingroup\$

Before implementing a GUI I tried performing it at CLI first and I also tried to implement it by using calling methods to another class.

BinaryConversion class:

import java.io.IOException;
import static java.lang.System.in;
import java.util.InputMismatchException;
import java.util.Scanner;

public class BinaryConversion{

    public static void main(String[] args) throws IOException {

        try (Scanner in = new Scanner(System.in)) {

            System.out.print("Enter given: ");
                int givenNum = in.nextInt();
            System.out.println("Binary: " + convert.getBinary(givenNum));
            System.out.println("Octal:  " + convert.getOctal(givenNum));
            System.out.println("Hex:    " + convert.getHex(givenNum));

        } catch(InputMismatchException e) {

            System.out.println("Looks like you entered a non integer value.");

        } finally {

            in.close();

        }
    }
}

Convert class:

public final class convert{

    private convert() {
        // removes the default constructor
    }

    public static String getBinary(int given) {
        char binNumbers[] = {'0','1'}; 
        String str = "";
        int rem;
            while (given > 0) {
                rem = given % 2;
                str = binNumbers[rem] + str;
                given /= 2;
            }
        return str;
    }

    public static String getOctal(int given) {
       char octalNumbers[] = {'0','1','2','3','4','5','6','7'}; 
       String str = "";
       int rem;
           while (given > 0) {
               rem = given % 8;
               str = octalNumbers[rem] + str;
               given /= 8;
            } 
       return str;
    }

    public static String getHex(int given) {
        char hexNumbers[] = {'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E','F'};
        String str = "";
        int rem;
            while(given > 0){
                rem = given % 16;
                str = hexNumbers[rem] + str;
                given /= 16;
            }
        return str;
    }
}

My questions are:

  • Did I missed something?
  • Is there a more efficient implementation of this program?
\$\endgroup\$
4
\$\begingroup\$

First: Please format your code more consistently.

The indention of the whileloop is very unusual. Better (and more common):

public static String getBinary(int given) {
    char binNumbers[] = {'0','1'}; 
    String str = "";
    int rem;
    while (given > 0) {
        rem = given % 2;
        str = binNumbers[rem] + str;
        given /= 2;
    }
    return str;
}

Next: Please use the default cases for identifiers. Classes should start upper case (Convert instead of convert).

Also use StringBuilder instead of String-concatenation:

public static String getBinary(int given) {
    char binNumbers[] = {'0','1'}; 
    StringBuilder sb = new StringBuilder();
    int rem;
    while (given > 0) {
        rem = given % 2;
        sb.append(binNumbers[rem]);
        given /= 2;
    }
    return sb.toString();
}

Ths will be a large performace gain. Your code basically translates to:

public static String getBinary(int given) {
    char binNumbers[] = {'0','1'}; 
    String str = "";
    int rem;
    while (given > 0) {
        rem = given % 2;
        //str = binNumbers[rem] + str;
        // Code generated by compiler for the above line of code
        StringBuffer sb = new StringBuffer();
        sb.append(binNumbers[rem]);
        sb.append(str);
        str = sb.toString();
        // End of generated code
        given /= 2;
    }
    return str;
}

As you can see the code generated many objects which have to be cleaned up afterwards by the Garbage Collector.

Hint:

You can use Integer.toString(given, 2) instead of your Convert.getBinary(given) for the same result.

|improve this answer|||||
\$\endgroup\$
2
\$\begingroup\$

I have already stated the obvious in my comment, so I'll focus on the other parts of your code.

It's nice that you know how to use try-with-resources, but that also means you don't need the finally clause.

Your convert class should be Converter (note the title case), and your comment here is misleading:

private convert() {
    // removes the default constructor
}

Yes, it's understood that utility classes often do not have a public constructor, so you should either mention something to that extent, or just put a dummy comment (more to suppress empty code block warnings for the latter):

private Converter() {
    // utility class do not need public constructor
}

As for your actual conversion implementations, note that they tend to follow the pattern:

  • declare a char[] array of valid outputs
  • perform a modulus based on the length of the char[] array
  • perform an integer division and loop till this yields 0

As such, you can probably have the following private static method to standardize this:

private static String doConvert(int input, char[] values) {
    int base = values.length; // 2, 8, 16
    int rem;
    // ...
}

// illustration
public static String toBinary(int given) {
    // can consider putting new char[]{'0','1'} as a static field too
    return doConvert(given, new char[]{'0','1'});
}
|improve this answer|||||
\$\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.