Is decimal, hexadecimal, octadecimal, binary converter efficient?

Question

I have made a method which takes two parameters: the value to convert and the base. Based upon those two parameters my method is adjusted (with several helper methods) to change the desired decimal number to a number with a different base.

Is this code practical and clean? If not, what should I change to make it better?

public String baseConversion(int base, int value){
    String finalResult = "";
    String tempResult = "";
    int countLength = 0, countFit = 0; // countLength counts the length of the tempResult string in order to know how many '0' to add. countFit counts how many times the tempValue fits into the current parameter 'value'
    int placeValue; // Records the place value. ex 245: '2' is in the hundreds place value
    int tempValue = 0;

    while (value > 0){
        placeValue = 1;
        // calculates the highest power that fits in the value (takes desired base into account)
        while (value > placeValue){
            placeValue*=base;
            countLength++;
        }
        if (value!=placeValue){
            countLength--; // subtract to account for the value in the tempResult that is not '0'
            placeValue/=base; // divide back as the previous while loop went one placeValue higher than needed in order to break out
        }
        //figures out how many times the placeValue can fit into the value
        while (tempValue+placeValue <= value){
            tempValue+=placeValue;
            countFit++;
        }
        if (base==16){
            tempResult += numConverter(countFit);
        }
        if (base==8 || base == 2 || base == 10){
            tempResult += String.valueOf(countFit);
        }
        // adds zeros to the rest of the string
        while (countLength > 0){
            tempResult+="0";
            countLength--;
        }

        // takes the current tempResult and combines with previous. ex combine 10000 with 100 = 10100
        finalResult = combineResults(finalResult, tempResult);

        value -= tempValue;            
        countFit=0;
        tempValue=0;
        tempResult="";
    }
    return finalResult;
}

Helper methods:

private String numConverter(int number){
    // only for hexadecimal
    if(number == 10){
        return "A";
    }else if (number == 11){
        return "B";
    }else if (number == 12){
        return "C";
    }else if (number == 13){
        return "D";
    }else if(number == 14){
        return "E";
    }else if (number == 15){
        return "F";
    }else {
        return String.valueOf(number);
    }
}

private String combineResults(String a, String b){
    String result = "";
    int indexA = 0, indexB = 0;
    int aTempLength = a.length(), bTempLength = b.length();
    if (a.length() > b.length()){
        while (aTempLength > bTempLength){
            result+=a.charAt(indexA);
            indexA++;
            aTempLength--;
        }
    }else {
        while (bTempLength > aTempLength){
            result+=b.charAt(indexB);
            indexB++;
            bTempLength--;
        }
    }
    while (aTempLength > 0){
        if (String.valueOf(a.charAt(indexA)).equals("0")){
            result+=b.charAt(indexB);
            indexA++;
            indexB++;
        }else {
            result+=a.charAt(indexA);
            indexA++;
            indexB++;
        }
        aTempLength--;
    }
    return result;
}

Answer 1

Disclaimer: I'm new here, so this might not be the type of review people give, but I'm trying to match it as best as I can.

Formatting

A comment explaining the purpose of combineResults(String a, String b) next to the method would be helpful. (Yes, there's one in the main method, but documentation next to the helpers is good, too.)

The same could be said for numConverter(int number)...

Other than that, indentation looks good, braces look fine, etc. :)

Algorithm

In general: You should be passing around StringBuilders or StringBuffers (or chars) instead of Strings. The former are mutable, while the latter isn't mutable. This will save some performance. (e.g. appending is better on memory with the builders and buffers, rather than with raw Strings.)

Main Algorithm

Ok... You took kinda an odd approach to converting bases. Rather than using all those while loops, you should focus on determining the next least significant digit by using the modulus operator followed by division.

For example, in pseudocode:

while(number > 0) {
  nextDigit = number % base; //Get the next digit using modulo
  number /= base; 
  // prepend nextDigit to result String Buffer
}

This makes the combineResults() method unneeded, as prepend is built-in to the StringBuilder.

NumConverter

In the spirit of StringBuilders vs. String, this should return a char rather than a String.

Your current code assumes that the only number ever fed to this method will be less than 16. This could be a problem if it's a larger library, or if you want to expand the bases to more than hexadecimal.

You could use a switch/case construct instead of the else if's...

Or, if you really want to shorten the code, you could do something like (note: this also implements the char return instead of String):

char numConverter(int digit) {
   if (num >= 16 || num < 0) //If we're not given a valid digit
     return 0; //Let the output know somehow...  could throw an exception instead.
   else if (digit <= 10) //If we're dealing with a "normal" digit
     return ((char)num + '0'); //... return the numeric equivalent
   else //We're in extended digit range: we need letters!
     return ((char)digit + 'A' - 10); //Return hex equivalent of 10-15 (A-F)
}

Answer 2

A few random notes:

1. It returns an empty string with 0 as input value. I guess it's a bug.

2. int countLength = 0, countFit = 0;
   

   I'd put the variable declarations to separate lines. From Code Complete, 2nd Edition, p759:

   With statements on their own lines, the code reads from top to bottom, instead of top to bottom and left to right. When you’re looking for a specific line of code, your eye should be able to follow the left margin of the code. It shouldn’t have to dip into each and every line just because a single line might contain two statements.

3. Horizontal scrolling doesn't make reading easier. You could put the comments in the line before the variables:

   // countLength counts the length of the tempResult string in order to know how many '0' to add. 
   int countLength = 0; 
   //countFit counts how many times the tempValue fits into the current parameter 'value'
   int countFit = 0; 
   

   Anyway, commenting is a smell. Clean Code by Robert C. Martin, p54:

   The proper use of comments is to compensate for our failure to express ourself in code. Note that I used the word failure. I meant it. Comments are always failures. We must have them because we cannot always figure out how to express ourselves without them, but their use is not a cause for celebration.

   The book has lots of hints which help to avoid this smell.

4. Try to minimize the scope of local variables. It's not necessary to declare them at the beginning of the method, declare them where they are first used. (Effective Java, Second Edition, Item 45: Minimize the scope of local variables)

   For example, tempResult could be declared inside the while loop:

   while (value > 0) {
       String tempResult = "";
       ...
   
   }
   

   It would make the tempResult = ""; statement unnecessary at the end of the loop.

5. If you don't know the already existing methods for these conversions in the JDK check them. Integer class has toOctalString(int i), toHexString(int i) and toBinaryString(int i). You can look into their source too.

Answer 3

If you are deliberately reinventing-the-wheel, please tag your question as such. Since you didn't tag it, I'm obligated to mention that you could just write

Integer.toString(value, base).toUpperCase();

---

I'll start with the helper methods. I believe that neither helper should be necessary.

private String numConverter(int number) is basically equivalent to Character.toUpperCase(Character.forDigit(digit, 16)). That would be static, and returns a char; both would be a good idea for your numConverter(). Also, you only use numConverter() for hexadecimal, but it could actually work just as well for any base up to and including 16.

combineResults() looks like an adder that operates on strings, without needing to support carrying. That's an odd approach to base conversion. You should be able to build the result by appending a digit at a time without needing this function at all.

Now, on to baseConversion() itself.

The function is a pure function that does not rely on any object state. Therefore, it should be declared static.

Personally, I feel that the parameters would be more natural in the reverse order (value, base).

You don't support negative inputs. That could be reasonable, but returning an empty string for such inputs is not. Throw an exception. Not supporting 0 as a value is a bit more surprising. You should be able to tweak your code to handle 0. Another surprise is that if base is not 2, 8, 10, or 16, the function returns an empty string. Again, either add support or add validation.

The big block of variable declarations at the top is a bad sign. You should declare your variables in the tightest scope possible. The "temp" variables certainly don't belong there.

This if-block, which conditionally undoes one iteration of the preceding while-loop, is a sign that the while-loop's termination condition is wrong.

if (value!=placeValue){
    countLength--; // subtract to account for the value in the tempResult that is not '0'
    placeValue/=base; // divide back as the previous while loop went one placeValue higher than needed in order to break out
}

---

If asked to keep to the spirit of the original code, here's how I would rewrite it:

public static String baseConversion(int base, int value) {
    if (base <= 0 || value < 0) {
        throw new IllegalArgumentException("Negative bases and values are not supported");
    }
    if (base < Character.MIN_RADIX) {
        throw new IllegalArgumentException("Base too small");
    }
    if (base > Character.MAX_RADIX) {
        throw new IllegalArgumentException("Base too large");
    }

    StringBuilder result = new StringBuilder();
    int placeValue;
    for (placeValue = 1; placeValue * base <= value; placeValue *= base) {}
    for (; placeValue > 0; placeValue /= base) {
        int digit = value / placeValue;
        value -= digit * placeValue;
        result.append(Character.forDigit(digit, base));
    }
    return result.toString().toUpperCase();
}