handle distribution with fractions more smartly

[evykassirer]

right now (3 / x^2 + x / (x^2 + 3)) * (x^2 + 3) turns into (3 / x^2 * x^2 + 9 / x^2 + x / (x^2 + 3) * x^2 + 3x / (x^2 + 3) which isn't super helpful

I think distribution should check to see if one of the groups we're multiplying contains fractions. If one (not both, not none) is, then multiply the non-fraction term into the numerators of the other term.

so here we'd have

(3 / x^2 + x / (x^2 + 3)) * (x^2 + 3) -> (3 *(x^2 + 3)) / x^2 + (x *(x^2 + 3)) / (x^2 + 3)

(this also helps fix an issue in #88)

[lexiross]

Hey @evykassirer - this one looks cool. Doing some digging now (and refreshing my algebra skills, lol)...

[evykassirer]

really hope what I wrote actually covers the cases well haha XD

and if it does, I think this change will be really great

thanks for digging into it :D want me to mark it assigned?

[lexiross]

Okay, I'm gonna try to rephrase to make sure I'm understanding right:

Given (a + b) * (c + d) where a or b or both are expressed as fractions, but c and d are not, instead of distributing them using the FOIL method (a*c + a*d + b*c + b*d), treat (c + d) as one term, so it would look like a*(c + d) + b*(c + d).

is that right? Also, if only a or only b is a fraction, should this rule apply?

[evykassirer]

almost! (at least, in what I was thinking)

If only a or only b yeah I think it should still apply.

I would say if a or b is a fraction, and both c and d are not fractions, we'd not do FOIL (as you said) and instead treat (c+d) as one term (like you said), but move it into the numerator of any fractions

e.g.

(a/x + b/y) * (c + d) => (a*(c+d))/x + (b*(c+d))/y

(a + b/y) * (c + d) => a*(c+d) + (b*(c+d))/y

(a + b) * (c + d) => a*c + a*d + b*c + b*d

how's that look?

[lexiross]

Oh yeah, that makes sense! :)

[evykassirer]

yay - thanks for helping us clarify!

[lexiross]

@evykassirer one more question before I dive in - does this seem like a reasonable place to add this logic? https://github.com/socraticorg/mathsteps/blob/master/lib/simplifyExpression/distributeSearch/index.js#L177

I was thinking of checking whether this new case applies before the if on line 180, which would become an if else. The only thing I'm a bit confused about is that intermediate step on line 180 and how this new logic interacts with it (since it looks like the code doesn't actually directly implement FOIL). Should it matter whether each group of terms have > 1 node in it?

[evykassirer]

yes! that looks like a good place

oh good point - I forgot about this being more general. The distribution code handles multiplying together any two list of terms - so it could do (a+b)(c+d) but also can do a(b+c) and (a+b+c+d)(e+f)

the general case of this rule, I think, is if one of the sums has fractions and the other one doesn't

also note that (c + d) * (a + b/y) => (c+d)*a + ((c+d)*b)/y -- that is to say, the fractions could be in the right sum (secondArgs) as well

sooo many edge cases haha, i'm starting to remember how annoying it was to write this code initially. hopefully this change won't be too complicated or confusing ^_^

lemme know what you think and also if you think of any ways to make it better!

[lexiross]

@evykassirer submitted a PR along those lines ^ let me know how that looks!

[aelnaiem]

This seems to have been completed a while ago! Going to close the issue :)