Skip to content
This repository has been archived by the owner on May 23, 2019. It is now read-only.

Fraction multiplied by constant #81

Open
lucasgruwez opened this issue Jul 17, 2017 · 4 comments
Open

Fraction multiplied by constant #81

lucasgruwez opened this issue Jul 17, 2017 · 4 comments

Comments

@lucasgruwez
Copy link

lucasgruwez commented Jul 17, 2017

Hi @nicolewhite,

I noticed that when using algebra.js, a fraction multiplied by a constant is displayed like so:

var expr = algebra.parse("x * 2/5")
console.log(expr.toString())
> 2/5x

That could be misinterpreted as 2/(5x). I think it would be more logical to display it as 2x/5, which is clearer.

Thank you for creating such a great library by the way!

Lucas

@Benjadahl
Copy link
Contributor

Hey Lucas,
I have created a mockup solution for this, however I am not sure of its quality or if it breaks anything else in the project. As many of the unit tests are based on the term.prototype.toString() function, which is the one I have changed. I do not really have time to go through all of the tests myself.

The change is made in my commit 450dba3.

The code works by checking if it is the last coefficient, if it is, it will run the block of code, that inserted the variables after the coef before, however it will run this in between numerator and denominator.

Not sure how @nicolewhite feels about this?
It does not represent how the internals of the library works as well as before, however it might be more userfriendly.

A quick solution for you @lucasgruwez might be to disable implicit multiplication in the toString function?
Else I am able to provide you with a built version of my fix, if you want to use it.

@lucasgruwez
Copy link
Author

Hi @Benjadahl,

I have looked at how ASCII math would render 2/5x, and it renders it in a way that is hard to misinterpret, that looks like this:

 2
--- x
 5

And it renders 2x/5 as something more like:

   x
2 ---
   5

I think that we can easily agree that the first render is more elegant, therefore, I think it should be an option, rather than a feature.

I will have a look at your code and check how it performs.

Thank you for making that fix.

Lucas

@nicolewhite
Copy link
Owner

I think this does make sense but it is a large breaking change, so would need to be released under a new major version, unfortunately. I will think about it more.

@lucasgruwez
Copy link
Author

@nicolewhite,

Thanks for the response, as I said before, you could add this as an option, so that existing code doesn't change the output

// Example of how this could be done

expr.toString(true) // 2x/5
expr.toString()        // 2/5x

Therefore, you might not need a major release, but a minor release could do.

Lucas

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants