Code Review Stack Exchange is a question and answer site for peer programmer code reviews. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I'm mostly concerned about edge cases, although I think I dealt with them all. Improvements on code quality would also be much appreciated.

/*
    printRadian(90) -> π/2
    printRadian(-90) -> -π/2
    printRadian(180) -> π
    printRadian(-180) -> -π
    printRadian(270) -> 3π/2
*/

function printRadian(degrees) {

    if(degrees % 180 == 0) {
        if(degrees == 180) {
            return "π";
        }
        else if(degrees == -180){
            return "-π";
        }else{
            return degrees / 180 + "π";
        }
    }

    var frac = degrees / 180;

    // 10 ** number of decimal places 
    var multiplier = Math.pow(10, frac.toString().split(".")[1].length);
    var numerator = frac * multiplier;
    var denominator = multiplier;

    //http://stackoverflow.com/a/17445304/896112 - Euclid's Algorithm
    function gcd(a, b) {
        return (!b)?a:gcd(b, a%b);
    }

    var g = gcd(numerator, denominator);

    numerator = numerator/g;
    denominator = denominator/g;

    if(numerator == 1){
        numerator = "π";
    }else if(numerator == -1){
        numerator = "-π";
    }else{
        numerator += "π"
    }

    if(denominator < 0){
        denominator = Math.abs(denominator);
        numerator = "-"+numerator;
    }

    return numerator +"/"+ denominator; 
}
share|improve this question
1  
printRadian(270) -> 2π/2 -> π is wrong, it should be 3π/2 – Caridorc Jan 9 at 18:27
    
Ok you fixed it in the comments, but does the code give the correct result for it? – Caridorc Jan 9 at 18:28
    
@Caridorc ah that was a typo, I added the comments after posting. Goes to show how good my math skills are :) – Carpetfizz Jan 9 at 18:29
up vote 13 down vote accepted

Your code looks more complicated than necessary and some of your special cases are really not needed.

I will use an incremental development to avoid unnecessary code:


The first version is just based on the formula, with no additional prettiness:

function printRadians(angle) {
  return angle + "π" + "/" + 180;
}

document.write([45, 90, 180, 270, 360, -45, -18].map(printRadians).join('  '));


Then we want to simplify the fraction, using, as you did, the gcd function:

function gcd(a, b) {
  return (b == 0)?a:gcd(b, a%b);
}

function printRadians(angle) {
  var _gcd = gcd(angle, 180)
  return angle / _gcd + "π" + "/" + 180 / _gcd;
}

document.write([45, 90, 180, 270, 360, -45, -18].map(printRadians).join('  '));


We are getting nearer, now we should take care of removing unnecessary ones:

function gcd(a, b) {
  return (b == 0)?a:gcd(b, a%b);
}

function printRadians(angle) {
  var _gcd = gcd(angle, 180)
  var numerator = angle / _gcd == 1 ? "" : angle / _gcd;
  var denominator = 180 / _gcd == 1 ? "" : "/" + (180 / _gcd);
  return numerator + "π" + denominator;
}

document.write([45, 90, 180, 270, 360, -45, -18].map(printRadians).join('  '));


And now let's put the minus in front of the numerator, not denominator:

function gcd(a, b) {
  return (b == 0)?a:gcd(b, a%b);
}

function printRadians(angle) {
  var _gcd = gcd(angle, 180);
  var numerator = angle / _gcd == 1 ? "" : angle / _gcd;
  var denominator = 180 / _gcd == 1 ? "" : "/" + Math.abs(180 / _gcd);
  return ((angle < 0 ? "-" : "") + numerator) + "π" + denominator;
}

document.write([45, 90, 180, 270, 360, -45, -18].map(printRadians).join('  '));

And we are done, just \$4\$ lines, with no lengthy ifs for special cases.

share|improve this answer
1  
Awesome, thank you so much! – Carpetfizz Jan 9 at 19:59
2  
+1 This is really nice example of incremental development. – Honza Brabec Jan 10 at 11:36

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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