Don't allow dividing by 0

[evykassirer]

This is blocked on our error infrastruction #129 -- let's use it as the first error that we implement with the new error structure

---

We don't check for this, which means division by 0 gives weird results.

This functionality would probably go in simplifyBasics, or in a separate DFS before that - since checking for dividing by zero is important before anything else is attempted.

I think (though feel free to approach this a different way) that dividing by zero would look like this:

• the divide by 0 function returns a NodeStatus.hasChanged object where newNode is actually the same as oldNode and the changeType is DIVIDE_BY_ZERO
• in simplifyExpression, in the function stepThrough, after each call to step, check if the changeType was DIVIDE_BY_ZERO -- if so, break out of the while loop

[hmaurer]

I am on this; will PR soon. How should we handle the 0/0 case? Return DIVIDE_BY_ZERO too?

[evykassirer]

yeah, I think I'm comfy having them give the same error. Sound ok to you?

[hmaurer]

Yes I think that makes sense! I'll PR later tonight.

[evykassirer]

Hey @hmaurer, I see you have a branch open for this and a commit - did you forget to put up a PR or did you want to work on it a bit more first?

[evykassirer]

@hmaurer ?

[hmaurer]

@evykassirer Sorry for the delay; I have opened a PR

edit: will do in 5min, I first need to merge master and there are conflicts :(

[evykassirer]

no problem at all, just wanted to make sure it wasn't forgotten about ^_^

[aelnaiem]

This is interesting, I wonder if there should be a NodeStatus for errors like this in general that we check, not just divide by zero.

[evykassirer]

@aelnaiem I was thinking about that too!

In the PR, I talked about:

oh, maybe a nodeStatus should have both a changeType and an errorType (default to null)? idk, still trying to figure out what this would look like long term

what do you think?

[aelnaiem]

Yea I think that's a good idea!

[evykassirer]

UPDATE: now related to #129 -- let's use it as the first error that we implement with the new error structure

I removed the small label, since this is more complicated now