search

google / mathsteps

Step by step math solutions for everyone
https://socratic.org
Apache License 2.0
2.12k stars 275 forks source link

Better handling of distribution with fractions #147

Closed lexiross closed 7 years ago

lexiross commented 7 years ago

Addresses #138

This is just a first stab - happy to change around anything that should be different!

evykassirer commented 7 years ago

woohoo! thanks

I'm studying for a test today, but hope to review this before end of day tomorrow :)

lexiross commented 7 years ago

thanks for the review @evykassirer! and thanks lol - the fat cat thing was quite fun and I had no idea so many people on the internet would be into it 😂

just updated - lmk if there's anything else I can do

lexiross commented 7 years ago

@evykassirer added the parenthesis step - but then ended up with an edge case for 2(x+3)/3, which should have solved to 2/3 x + 2 but was instead 2x / 3 + 2. I decided to only do the parentheses check if the denominator of the fraction we're working with is NOT a constant. Does that sound right?

evykassirer commented 7 years ago

Hmmm yeah we should maybe standardize if we do 2/3 x or 2x / 3 but I believe both can show up in the solver and I don't really mind if it ends up one way or the other. I'll open an issue to discuss that!

So I think it's alright either way you do it (though I have slight preference for always adding the parens)

Thanks for your patience this week btw :) It's been really heavy workload for me so I haven't been able to be on GitHub as much, but you're bringing up lots of good points and also this diff is great! :D

lexiross commented 7 years ago

Okay! So if we do the parentheses all the time, it's okay to change the existing test?

And no worries at all!! I'm in no rush :)

evykassirer commented 7 years ago

Yeah, I don't think it changes the correctness or pedagogy, so go for it :)

(if you want - if you'd like to keep it as is, just lemme know and I'll merge away!)

lexiross commented 7 years ago

Okay, updated :)