Some additional helper functions #7
Conversation
| ] | ||
| `; | ||
|
|
||
| exports[`query getVariableFactors 2x^2 * z^2 + y^3 1`] = ` |
There was a problem hiding this comment.
y^3 is not a factor, it's a term. If you want a function that finds all variable factors we should have a separate function for that.
aliang8
changed the title
lib/__test__/query.test.js
Outdated
| ]) | ||
|
|
||
| it('isVariableFactor', () => { | ||
| it('isVariableFactor', () => { | ||
| console.log(query.isPolynomial(parse('2*(y+x)'))) |
There was a problem hiding this comment.
I think these two lines should be reverted.
lib/query.js
Outdated
| @@ -232,7 +232,7 @@ export const getCoefficientsAndConstants = (node, coefficientMap = {}, constants | |||
| } else { | |||
| coefficientMap[key].push(coefficient) | |||
| } | |||
| } else if(isPolynomial(node) || isApply(node)) { | |||
| } else if(isPolynomial(node) || isAdd(node) || (isMul(node) && node.args.every(isPolynomialTerm))) { | |||
| // 2x^2 + 3x + 1, 3x^2 * 2x^2 | |||
There was a problem hiding this comment.
Given the comment, shouldn't checking isPolynomial(node) enough since we've already ruled out constants and single term polynomials. Why are the other conditions necessary?
There was a problem hiding this comment.
I don't think so; it leaves out non-polynomial expressions such as x + 4 + x + 2^x that we would still want to collect and combine.
There was a problem hiding this comment.
Gotcha. Can you update the comment with that example?
aliang8
changed the title
| @@ -41,25 +41,25 @@ const relationIdentifierMap = { | |||
|
|
|||
| export const isRel = node => isApply(node) && node.op in relationIdentifierMap | |||
|
|
|||
| export const isNumber = node => { | |||
There was a problem hiding this comment.
I think we should keep isNumber in addition to isRationalNumber.
There was a problem hiding this comment.
Hmmm I don't see how it would be useful, but I add it back.
There was a problem hiding this comment.
I guess we isInteger. Okay, let's leave this as is. We can always add it back later if it does turn out we need it.
| assert(query.isNumber(a)) | ||
| assert(query.isNumber(parse('-2'))) | ||
| assert(!query.isNumber(x)) | ||
| it('isRationalNumber', () => { |
There was a problem hiding this comment.
Maybe add some tests that check for factions and decimals.
lib/query.js
Outdated
| } | ||
|
|
||
| // e.g 3 -> -3, -3x -> 3x, x + 3 -> -(x + 3), 2/3 -> -2 / 3 | ||
| export const negate = (node) => { |
There was a problem hiding this comment.
I think this should be part of build not query.
getVariableFactors
2x^2 * ygetCoefficientsAndConstants
x + 4 + 2^x + 4negate
-3x -> 3xhasCoeff
2x^2 -> true, x -> false