This repository was archived by the owner on Aug 29, 2024. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 279
#76 - Better break-up fraction. #158
Open
eltonlaw
wants to merge
11
commits into
google:master
Choose a base branch
from
eltonlaw:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
aee08a1
Issue #76 - Better break-up fraction.
eltonlaw 6135dad
add check for denonimator in numerator
eltonlaw 322a247
Issue #76 - Better break-up fraction
eltonlaw 3c1cac0
lint
eltonlaw 1ccdc3b
added test cases, checks are more rigorous
eltonlaw a46849d
Merge branch 'master' into master
eltonlaw fd8ebf0
add comments
eltonlaw f779651
update
eltonlaw 41e9223
squash line 55-56 into one if statement
eltonlaw 308a90c
Add changes
eltonlaw d88605f
Add TODO's
eltonlaw File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
const Node = require('../node'); | ||
|
||
// Returns true if by adding a term you can simplify part of the function into | ||
// an integer | ||
// e.g. (2x+1)/(2x+3) -> True because of the following simplification | ||
// (2x+1)/(2x+3) -> (2x + 3)/(2x + 3) - 2/(2x + 3) -> 1 - 2/(2x + 3) | ||
// e.g. (2x+1)/(2x^2 + 3) -> False | ||
// ============================================================================== | ||
// CHECKS | ||
// - Check for division in parent node | ||
// - Numerator has to be addition/subtraction of a polynomial term to the power | ||
// of 1 and a constant term, in that order OR a polynomial term to the | ||
// power of 1 | ||
// - Denominator has to be addition/subtraction of a polynomial term to the power | ||
// of 1 and a constant term, in that order. | ||
// - Check to see that the denominator and numerator have the same symbol | ||
|
||
function canFindDenominatorInNumerator(node) { | ||
if (node.op !== '/' ) { | ||
return false; | ||
} | ||
let numerator = node.args[0]; | ||
let denominator = node.args[1]; | ||
if (Node.Type.isParenthesis(numerator)) { | ||
numerator = numerator.content; | ||
} | ||
if (Node.Type.isParenthesis(denominator)) { | ||
denominator = denominator.content; | ||
} | ||
|
||
let numeratorArgsLength; | ||
// If numerator has args, but it's just a polynomial term, length is 1 | ||
// Ex. 3x/2x+3 => numeratorArgsLength=1 | ||
if (Node.PolynomialTerm.isPolynomialTerm(numerator)) { | ||
numeratorArgsLength = 1; | ||
} | ||
// If numerator has args and args are two seperate values length is 2 | ||
// Ex. 3x+4/2x+3 => numeratorArgsLength=2 | ||
else if (numerator.op === '+' || numerator.op === '-') { | ||
numeratorArgsLength = numerator.args.length; | ||
} | ||
// If numerator doesn't have args and isn't a polynomial term, there's | ||
// nothing the added functionality can do | ||
// Ex. 3/(2x + 3) => False | ||
else { | ||
return false; | ||
} | ||
let denominatorArgsLength; | ||
if (denominator.op === '+' || denominator.op === '-') { | ||
denominatorArgsLength = denominator.args.length; | ||
} | ||
// If denominator doesn't have args, it's length is 1. This case is already | ||
// resolved by splitting the denominator into all the numerators | ||
// Ex. (x + 3)/2x => x/2x + 3/2x | ||
else { | ||
return false; | ||
} | ||
// Function doesn't support denominators with args > 2 | ||
if (denominatorArgsLength !== 2) { | ||
return false; | ||
} | ||
// Check if numerator's second argument is a constant if numerator has two arguments | ||
if (numeratorArgsLength === 2) { | ||
if (!Node.Type.isConstant(numerator.args[1])) { | ||
return false; | ||
} | ||
} | ||
// Check if denominator's second argument is a constant | ||
if (!Node.Type.isConstant(denominator.args[1])) { | ||
return false; | ||
} | ||
// Defines the first term depending on whether there's a coefficient value | ||
// with the first term | ||
let numeratorFirstTerm; | ||
if (numerator.op === '+') { | ||
numeratorFirstTerm = new Node.PolynomialTerm(numerator.args[0]); | ||
} | ||
else { | ||
numeratorFirstTerm = new Node.PolynomialTerm(numerator); | ||
} | ||
let denominatorFirstTerm; | ||
if (denominator.op === '+') { | ||
denominatorFirstTerm = new Node.PolynomialTerm(denominator.args[0]); | ||
} | ||
// If an exponent exists (aka not x^1), return false | ||
if (numeratorFirstTerm.getExponentNode() || | ||
denominatorFirstTerm.getExponentNode()) { | ||
return false; | ||
} | ||
// Check that the symbols are the same, Ex. (x+1)/(y+1) would not pass | ||
if (!(numeratorFirstTerm.getSymbolName() === | ||
denominatorFirstTerm.getSymbolName())) { | ||
return false; | ||
} | ||
return true; | ||
} | ||
|
||
module.exports = canFindDenominatorInNumerator; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,3 +29,26 @@ describe('canSimplifyPolynomialTerms addition', function() { | |
]; | ||
tests.forEach(t => testCanCombine(t[0], t[1])); | ||
}); | ||
|
||
describe('canSimplifyPolynomialTerms denominator in numerator', function() { | ||
const tests = [ | ||
['(x+1)/(x-2)', true], | ||
['(2x)/(x+4)', true], | ||
['(x)/(x+4)', true], | ||
['(x)/(2x+4)', true], | ||
['(x+3)/(x)', false], // Normal breakup function already solves this | ||
['(2x + 3)/(2x + 2)', true], | ||
['(2x+3)/(2x)', false], // Normal breakup function already solves this | ||
['(2x)/(2x + 2)', true], | ||
['(5x + 3)/(4)', false], // Normal breakup function already solves this | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. these comments are super helpful! |
||
// Not supported yet | ||
['(2x)/(2 + 2x)', false], | ||
['(2 + 2x)/(3x + 4)', false], | ||
['(x + 3)/(2x^2 + 5)', false], | ||
['(3x^2 + 3)/(2x^2 + 5)', false], | ||
['(5x^2 + 3)/(2x + 5)', false], | ||
['(5x^2-4x + 3)/(2x + 5)', false], | ||
['(-4x + 3)/(2x^2 + 5x +7)', false], | ||
]; | ||
tests.forEach(t => testCanCombine(t[0], t[1])); | ||
}); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be best if we can break this up into two steps. the step you added, and the break up fraction that comes after (and then maybe we can group in the cancelling that comes after). and then we'd have substeps for breaking up the fraction.
what do you think? (let me know if you want me to explain more what this means)