Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

What is the meaning of separateExpression and isSeparable? #399

Open
SergeStinckwich opened this issue Dec 15, 2021 · 3 comments
Open

What is the meaning of separateExpression and isSeparable? #399

SergeStinckwich opened this issue Dec 15, 2021 · 3 comments

Comments

@SergeStinckwich
Copy link
Collaborator

SergeStinckwich commented Dec 15, 2021

The semantic of the separateExpression method on KEBinaryExpression is not really clear.
Tests should give the semantic more clearly and the method should be renamed appropriately.

testSeparateABinaryExpression

	| s1 s2 s3 s4 |
	s1 := 's=-(A+B+C)*E*D' parseAsAnEquation.
	s2 := 's=-(A+B)*(-C)' parseAsAnEquation.
	s3 := 's=(-E)*D*F*(A+B)' parseAsAnEquation.
	s4 := 's=(A+B)*(C+D)' parseAsAnEquation.
	self assert: 3 equals: s1 expression separateExpression size.
	self assert: 2 equals: s2 expression separateExpression size.
	self assert: 2 equals: s3 expression separateExpression size.
	self assert: 4 equals: s4 expression separateExpression size

The same for the method isSeparable.

@SergeStinckwich SergeStinckwich changed the title What is the meaning of separateExpression What is the meaning of separateExpression Dec 15, 2021
@SergeStinckwich SergeStinckwich changed the title What is the meaning of separateExpression What is the meaning of separateExpression and isSeparable? Dec 15, 2021
@SergeStinckwich
Copy link
Collaborator Author

SergeStinckwich commented Dec 15, 2021

separateExpression is used only in generateEvents method on KEExpression.
After a while, I was able to understand that separateExpression is about being able to express an expression as a sum of factors (the reverse of the factorization).

We need to replace this method with an expand method on KEExpression.

@mikalziane
Copy link
Collaborator

the form of the test is not appropriate. One should be able to express 2 different things in tests
a) expr 1 = expr 2
b) f(expr1) == expr 2 where == means identical in form

@SergeStinckwich
Copy link
Collaborator Author

I refactor the test as:

testSeparateABinaryExpression

	| e1 e2 e3 e4 |
	e1 := '-(A+B+C)*E*D' parseAsAnExpression.
	e2 := '-(A+B)*(-C)' parseAsAnExpression.
	e3 := '(-E)*D*F*(A+B)' parseAsAnExpression.
	e4 := '(A+B)*(C+D)' parseAsAnExpression.
	self assert: e1 separateExpression size equals: 3.
	self assert: e2 separateExpression size equals: 2.
	self assert: e3 separateExpression size equals: 2.
	self assert: e4 separateExpression size equals: 4

More refactorings are needed in order to remove the use of Strings.

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

No branches or pull requests

2 participants