feat: add decimal to fraction algorithm #1439

MarufHasan24 · 2023-10-05T01:30:38Z

Describe your change:

Add an algorithm?
Fix a bug or typo in an existing algorithm?
Documentation change?

Checklist:

I have read CONTRIBUTING.md.
This pull request is all my own work -- I have not plagiarized.
I know that pull requests will not be merged if they fail the automated tests.
This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
All new JavaScript files are placed inside an existing directory.
All filenames should use the UpperCamelCase (PascalCase) style. There should be no spaces in filenames.
Example:UserProfile.js is allowed but userprofile.js,Userprofile.js,user-Profile.js,userProfile.js are not
All new algorithms have a URL in their comments that points to Wikipedia or another similar explanation.
If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

appgurueu

Please make clear what the spec of this is. What is the accuracy? If it's in decimal places, why do you need any complicated logic beyond rounding, and choosing the appropriate power of ten for the denominator?

Maths/Fraction.js

MarufHasan24

Maybe I have answered all your questions, and resolved those, what I think I should.

Maths/Fraction.js

appgurueu · 2023-10-10T21:59:59Z

I'd like to reiterate this:

Please make clear what the spec of this is. What is the accuracy? If it's in decimal places, why do you need any complicated logic beyond rounding, and choosing the appropriate power of ten for the denominator?

Code-wise, it seems you misunderstood my point. I'm asking for a "flattening" of the code using early returns / throws. The logic should be something like "if input is invalid, throw. if input is trivial (integer), return early. now do the actual meaty logic (if we got here, we didn't return or throw already)".

MarufHasan24 · 2023-10-11T02:59:10Z

I'd like to reiterate this:

Please make clear what the spec of this is. What is the accuracy? If it's in decimal places, why do you need any complicated logic beyond rounding, and choosing the appropriate power of ten for the denominator?

Code-wise, it seems you misunderstood my point. I'm asking for a "flattening" of the code using early returns / throws. The logic should be something like "if input is invalid, throw. if input is trivial (integer), return early. now do the actual meaty logic (if we got here, we didn't return or throw already)".

If we got the number which is not an integer, then what!? Or if the is input is not a number even, then I will wait for what? Please clear me.😟

appgurueu · 2023-10-12T17:20:54Z

If we got the number which is not an integer, then what!? Or if the is input is not a number even, then I will wait for what? Please clear me.😟

The logic should stay the same, it's just about how you write it. Currently the code is a mess of if-elses - e.g., elses to throw in error cases. What I'm proposing is to use early returns and throws to clean it up structure-wise. Suppose you have

function decimalToFraction(decimal) {
    if (typeof decimal === "number") {
        if (Number.isInteger(decimal)) {
             return [decimal, 1]
        } else {
             // this is where the logic for converting a valid decimal that isn't an integer to a fraction goes...
        }
    } else {
        throw "invalid type"
    }
}

then this would be better written as

function decimalToFraction(decimal) {
    if (typeof decimal !== "number") throw "invalid type"
    if (Number.isInteger(decimal)) return [decimal, 1]
    // this is where the logic for converting a valid decimal that isn't an integer to a fraction goes...
}

MarufHasan24 · 2023-10-13T01:22:08Z

@appgurueu fixed all issues you have mentioned.

MarufHasan24 · 2023-10-30T01:39:36Z

@appgurueu any progress?!

raklaptudirm · 2023-11-08T03:45:14Z

@appgurueu waiting for your updated review.

appgurueu

I don't see this robustly implementing (or even precisely defining) the problem yet; the code is also currently still quite dirty.

appgurueu · 2023-11-10T15:30:49Z

Maths/Fraction.js

+ * @function fraction
+ * @description This function returns the fraction of a float type number.
+ * @param {number} num - a float type number is expected, but an integer will also work.
+ * @param {number} accuracy - the accuracy of the fraction, the default is 6. like if the accuracy is 2 then for 1.333 result will be 10/9, where for 6 it will return 1333/1000.


Please formulate this mathematically - something like "fraction - num <= 10^-accuracy".

appgurueu · 2023-11-10T15:31:37Z

Maths/Fraction.js

+ * @see https://en.wikipedia.org/wiki/Repeating_decimal and 
+ * @see https://en.wikipedia.org/wiki/Decimal#Decimal_fractions
+ * @example fraction(0.5) // [1, 2]
+ * @example fraction(0.3333333333333333) // [1, 3]


Are all these examples really useful? 0.5 and 0.25 for example seem redundant.

appgurueu · 2023-11-10T15:31:49Z

Maths/Fraction.js

+ * @example fraction(0.33,2) // [10, 3] 
+ */
+function fraction(number, accuracy = 6) {
+    if (typeof number === "number" && !Number.isNaN(number) && Number.isFinite(number)


You're still not using guards here; these checks are all redundant with the if-throws in the else. You just need to move the if-throws to the top of the function, making them guards.

appgurueu · 2023-11-10T15:34:03Z

Maths/Fraction.js

+        len /= div;
+        return [neg * number, len];
+    } else {
+        if (typeof number !== "number") throw new TypeError("Invalid number, a number type value expected");


These "guards" should all be moved to the top of the function. They could also be simplified a bit.

appgurueu · 2023-11-10T15:34:16Z

Maths/Fraction.js

+    } else {
+        if (typeof number !== "number") throw new TypeError("Invalid number, a number type value expected");
+        if (typeof accuracy !== "number") throw new TypeError("Invalid accuracy, a number type value expected");
+        if (Number.isNaN(number)) throw new TypeError("Invalid number, a number type value expected");


The NaN check is redundant with the negated isFinite check.

appgurueu · 2023-11-10T15:40:38Z

Maths/Fraction.js

+            len = 10 ** (number.length - 1);
+            number = Number(number);
+        }
+        // it will find out the gcd of the number and the len to reduce the fraction nomitor and denominator like 4/8 will be 1/2


This is called "shortening" the fraction. number and len are odd names for "numerator" and "denominator".

appgurueu · 2023-11-10T15:50:26Z

Maths/Fraction.js

+                "9".repeat(reg[1].length) + "0".repeat(rec.length - pos[0].length)
+            );
+        } else {
+            // if number is not a repeating decimal then following code will run


Again a redundant comment.

appgurueu · 2023-11-10T15:52:30Z

Maths/Fraction.js

+        } else {
+            // if number is not a repeating decimal then following code will run
+            number = number.replace(".", "");
+            len = 10 ** (number.length - 1);


It is not apparent to me why this is correct: Isn't number.length the length of the whole number (without the period, in digits), not just the "fractional" part after the period here?

appgurueu · 2023-11-10T15:53:04Z

Maths/Fraction.js

+            );
+        } else {
+            // if number is not a repeating decimal then following code will run
+            number = number.replace(".", "");


String manipulation for manipulating numbers is dirty here. Just multiply your numerator by the denominator, then round. Shouldn't the denominator be determined by the accuracy?

appgurueu · 2023-11-10T15:57:08Z

Maths/Fraction.js

+        // if number is a not an integer then following code will run
+        number = number.toString();
+        let len;
+        let reg = number.match(/(\d+?)\1+$/);


This is a hacky way to deal with "repeating" fractional parts, and I'd argue that it solves the wrong problem. Since we're dealing with finite-size numbers (floats) here, there are no "repeating" decimals; we can't distinguish a "repeating" decimal from a "deliberately ending decimal". We need to treat both the same (it seems "accuracy" is more of a "threshold" of when you consider a decimal repeating?).

In fact, all floats can even be represented exactly as fractions (though perhaps maybe not as fractions of floats, that might get hairy towards some edge cases) where the denominator is a power of two and the numerator is an integer.

I wouldn't exclude the possibility that some special provisions for repeating decimals may be useful, but then what problem this exactly solves (and how) and what guarantees it makes should be explicitly stated.

MarufHasan24 added 3 commits October 5, 2023 07:26

Create Fraction.js

a5f3c3f

Create Fraction.test.js

e240332

Update Fraction.js

6dbea14

MarufHasan24 requested review from raklaptudirm and appgurueu as code owners October 5, 2023 01:30

RRMovies approved these changes Oct 9, 2023

View reviewed changes

appgurueu reviewed Oct 9, 2023

View reviewed changes

Maths/Fraction.js Outdated Show resolved Hide resolved

Maths/Fraction.js Outdated Show resolved Hide resolved

Maths/Fraction.js Outdated Show resolved Hide resolved

Maths/Fraction.js Outdated Show resolved Hide resolved

MarufHasan24 added 2 commits October 10, 2023 11:11

Update Fraction.js

2abb00d

fixing spelling Fraction.js

560127f

MarufHasan24 commented Oct 10, 2023

View reviewed changes

Maths/Fraction.js Outdated Show resolved Hide resolved

Update Fraction.js

82fd7fc

MarufHasan24 added 5 commits October 13, 2023 06:56

Updated Fraction.js

be441e7

Update spelling in Fraction.js

1e2000d

Oh! spelling.😓

aa71162

Update Fraction.test.js

c33f80e

logical error fixed.

ca69ae3

appgurueu requested changes Nov 10, 2023

View reviewed changes

MarufHasan24 closed this Apr 11, 2025

Uh oh!

feat: add decimal to fraction algorithm #1439

feat: add decimal to fraction algorithm #1439

Uh oh!

Conversation

MarufHasan24 commented Oct 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Describe your change:

Checklist:

Uh oh!

appgurueu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MarufHasan24 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

appgurueu commented Oct 10, 2023

Uh oh!

MarufHasan24 commented Oct 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

appgurueu commented Oct 12, 2023

Uh oh!

MarufHasan24 commented Oct 13, 2023

Uh oh!

MarufHasan24 commented Oct 30, 2023

Uh oh!

raklaptudirm commented Nov 8, 2023

Uh oh!

appgurueu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

MarufHasan24 commented Oct 5, 2023 •

edited

Loading

MarufHasan24 commented Oct 11, 2023 •

edited

Loading