Find Common Denominator for 2 Fractions with JavaScript - Simplify & Improve

Question 1

I wrote a JavaScript that takes 2 fractions and finds the common denominator for both. If the user inputs 2/6 and 1/2 --- the script outputs 2/6 + 3/6. I'm interested in feedback on possibly simplifying it or improving it. Thanks.

JS Bin: http://jsbin.com/omelen/1/edit

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
<title>Untitled Document</title>
<script>
window.onload = function() {
 // Finds the highest common factor of 2 numbers
 function highestCommonFactor(a, b) {
 if (b == 0) {
 return a;
 }
 return highestCommonFactor(b, (a % b));
 }
 // Input fractions to add ////////////////////////////////
 // Fraction 1 = 2/6
 var fraction1Numerator = 2;
 var fraction1Denominator = 6;
 // Fraction 2 = 1/2
 var fraction2Numerator = 1;
 var fraction2Denominator = 2;
 ////////////////////////////////////////////////////////////////
 // Find the highest common factor of both denominators
 var factor = highestCommonFactor(fraction1Denominator, fraction2Denominator); // 2
 if (factor > 1) { // There's a common factor greater than 1
 if (fraction1Denominator > fraction2Denominator) {
 // Find out how many times the factor goes into bigger denominator
 var temp = fraction1Denominator / factor;
 fraction2Numerator = fraction2Numerator * temp;
 fraction2Denominator = fraction2Denominator * temp;
 } else {
 // Find out how many times the factor goes into bigger denominator
 var temp = fraction2Denominator / factor;
 fraction1Numerator = fraction1Numerator * temp;
 fraction1Denominator = fraction1Denominator * temp;
 }
 } else { // There's no common factor greater than 1 so we need to multiple each fraction by each others denominators
 // Temp values
 var fraction1NumeratorTemp = fraction1Numerator;
 var fraction1DenominatorTemp = fraction1Denominator;
 fraction1Numerator = fraction1Numerator * fraction2Denominator;
 fraction1Denominator = fraction1Denominator * fraction2Denominator;
 fraction2Numerator = fraction2Numerator * fraction1DenominatorTemp;
 fraction2Denominator = fraction2Denominator * fraction1DenominatorTemp;
 }
 // Display solution
 document.getElementById("divSolution").innerText = fraction1Numerator + "/" + fraction1Denominator + " + " + fraction2Numerator + "/" + fraction2Denominator;
}
</script>
</head>
<body>
<div id="divSolution"></div>
</body>
</html>

Question 2

Here's my attempt to help you.

As a general rule, keep everything as general as you can and don't split your code into different cases unless you have to. From the if (factor >1) part, I knew things could be improved
At the end, the denominators will be the Least Common Multiple of the two initial fractions.
Then, you just need to multiply the numerators by the same amount used to multiply the denominators

Here's the corresponding code.

// Finds the highest common factor of 2 numbers
function highestCommonFactor(a, b) {
 if (b == 0) {
 return a;
 }
 return highestCommonFactor(b, a%b);
}
function leastCommonMultiple(a,b) {
 return a*b/(highestCommonFactor(a,b));
}
// Input fractions to add ////////////////////////////////
// Fraction 1 = 2/6
var fraction1Numerator = 2;
var fraction1Denominator = 6;
// Fraction 2 = 1/2
var fraction2Numerator = 1;
var fraction2Denominator = 2;
////////////////////////////////////////////////////////////////
// Find the highest common factor of both denominators
var commonMultiple = leastCommonMultiple(fraction1Denominator, fraction2Denominator); 
fraction1Numerator *= (commonMultiple / fraction1Denominator);
fraction2Numerator *= (commonMultiple / fraction2Denominator);
// Display solution
document.getElementById("divSolution").innerText = fraction1Numerator + "/" + commonMultiple + " + " + fraction2Numerator + "/" + commonMultiple;

Question 3

Thanks for taking a look at my code. I tried your revised code but had some issues... I inputted the following fractions and got the following incorrect results...Fraction 1 = 1/2 --> 1/6 Fraction 2 = 2/6 --> 6/6; Fraction 1 = 2/12 --> 2/36 Fraction 2 = 1/18 --> 6/36; Fraction 1 = 1/18 --> 1/36 Fraction 2 = 2/12 --> 12/26.

Question 4

I changed the following 2 lines of your code: fraction2Numerator *= (commonMultiple / fraction1Denominator); fraction2Numerator *= (commonMultiple / fraction2Denominator); --- TO ---> numeratorMultipler1 = commonMultiple / fraction1Denominator; numeratorMultipler2 = commonMultiple / fraction2Denominator; fraction1Numerator = numeratorMultipler1 * fraction1Numerator; fraction2Numerator = numeratorMultipler2 * fraction2Numerator; And that fixed the issue. Let me know what you think?

Question 5

Actually, you just need to change : fraction2Numerator *= (commonMultiple / fraction1Denominator); for fraction1Numerator *= (commonMultiple / fraction1Denominator); (one character is to be changed)

Question 6

Also, I think before programming anything, you should try to do your calculation with a pen and paper and try to figure out what you are doing. Then, once everything's clear, you can open your favorite text editor.

Question 7

I see it now. Thanks. I started learning JavaScript a few months ago, I appreciate your feedback, especially on how I can make my code clearer and more efficient. I plan on posting a few more similar fraction/decimal codes for review and would like your feedback if you have time.

SylvainD SylvainD 29.7k1 gold badge49 silver badges93 bronze badges · Accepted Answer · 2013-01-09 02:32:10Z

Here's my attempt to help you.

As a general rule, keep everything as general as you can and don't split your code into different cases unless you have to. From the if (factor >1) part, I knew things could be improved
At the end, the denominators will be the Least Common Multiple of the two initial fractions.
Then, you just need to multiply the numerators by the same amount used to multiply the denominators

Here's the corresponding code.

// Finds the highest common factor of 2 numbers
function highestCommonFactor(a, b) {
 if (b == 0) {
 return a;
 }
 return highestCommonFactor(b, a%b);
}
function leastCommonMultiple(a,b) {
 return a*b/(highestCommonFactor(a,b));
}
// Input fractions to add ////////////////////////////////
// Fraction 1 = 2/6
var fraction1Numerator = 2;
var fraction1Denominator = 6;
// Fraction 2 = 1/2
var fraction2Numerator = 1;
var fraction2Denominator = 2;
////////////////////////////////////////////////////////////////
// Find the highest common factor of both denominators
var commonMultiple = leastCommonMultiple(fraction1Denominator, fraction2Denominator); 
fraction1Numerator *= (commonMultiple / fraction1Denominator);
fraction2Numerator *= (commonMultiple / fraction2Denominator);
// Display solution
document.getElementById("divSolution").innerText = fraction1Numerator + "/" + commonMultiple + " + " + fraction2Numerator + "/" + commonMultiple;

Thanks for taking a look at my code. I tried your revised code but had some issues... I inputted the following fractions and got the following incorrect results...Fraction 1 = 1/2 --> 1/6 Fraction 2 = 2/6 --> 6/6; Fraction 1 = 2/12 --> 2/36 Fraction 2 = 1/18 --> 6/36; Fraction 1 = 1/18 --> 1/36 Fraction 2 = 2/12 --> 12/26.
I changed the following 2 lines of your code: fraction2Numerator *= (commonMultiple / fraction1Denominator); fraction2Numerator *= (commonMultiple / fraction2Denominator); --- TO ---> numeratorMultipler1 = commonMultiple / fraction1Denominator; numeratorMultipler2 = commonMultiple / fraction2Denominator; fraction1Numerator = numeratorMultipler1 * fraction1Numerator; fraction2Numerator = numeratorMultipler2 * fraction2Numerator; And that fixed the issue. Let me know what you think?
Actually, you just need to change : fraction2Numerator *= (commonMultiple / fraction1Denominator); for fraction1Numerator *= (commonMultiple / fraction1Denominator); (one character is to be changed)
Also, I think before programming anything, you should try to do your calculation with a pen and paper and try to figure out what you are doing. Then, once everything's clear, you can open your favorite text editor.
I see it now. Thanks. I started learning JavaScript a few months ago, I appreciate your feedback, especially on how I can make my code clearer and more efficient. I plan on posting a few more similar fraction/decimal codes for review and would like your feedback if you have time.

Stack Exchange Network

Find Common Denominator for 2 Fractions with JavaScript - Simplify & Improve

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Hot Network Questions

Find Common Denominator for 2 Fractions with JavaScript - Simplify & Improve

1 Answer 1

Your Answer

Sign up or log in

Post as a guest

Post as a guest

Related

Hot Network Questions