JavaScript icon indicating copy to clipboard operation
JavaScript copied to clipboard

feat: add decimal to fraction algorithm

Open MarufHasan24 opened this issue 2 years ago • 6 comments

Open in Gitpod know more

Describe your change:

  • [x] Add an algorithm?
  • [ ] Fix a bug or typo in an existing algorithm?
  • [ ] Documentation change?

Checklist:

  • [x] I have read CONTRIBUTING.md.
  • [x] This pull request is all my own work -- I have not plagiarized.
  • [x] I know that pull requests will not be merged if they fail the automated tests.
  • [x] This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • [x] All new JavaScript files are placed inside an existing directory.
  • [x] 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
  • [x] All new algorithms have a URL in their comments that points to Wikipedia or another similar explanation.
  • [x] If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

MarufHasan24 avatar Oct 05 '23 01:10 MarufHasan24

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)".

appgurueu avatar Oct 10 '23 21:10 appgurueu

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.😟

MarufHasan24 avatar Oct 11 '23 02:10 MarufHasan24

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...
}

appgurueu avatar Oct 12 '23 17:10 appgurueu

@appgurueu fixed all issues you have mentioned.

MarufHasan24 avatar Oct 13 '23 01:10 MarufHasan24

@appgurueu any progress?!

MarufHasan24 avatar Oct 30 '23 01:10 MarufHasan24

@appgurueu waiting for your updated review.

raklaptudirm avatar Nov 08 '23 03:11 raklaptudirm