cynic-parser: implement PartialEq for Type and Value #924
Conversation
I am conflicted on whether this should be PartialEq or a method, but since
there is only one useful way to compare {types,values}, I went with
PartialEq, as well as Eq for types (Value has a Float variant).
✅ Deploy Preview for cynic-querygen-web canceled.
|
| (Value::Int(a), Value::Int(b)) => a == b, | ||
| (Value::Variable(a), Value::Variable(b)) => a == b, | ||
| (Value::Float(a), Value::Float(b)) => a == b, | ||
| (Value::String(a), Value::String(b)) => a == b, |
There was a problem hiding this comment.
I was surprised to see no Value::BlockString variant in executable. Is it because the difference is less relevant there, or an omission?
There was a problem hiding this comment.
This is kind of intentional, though now that you point it out I am not sure it makes sense.
When I added support for block string processing I noticed it was incredibly slow - like a good 20-30% of schema parsing was spent processing block strings. So I made the processing of them lazy for schema parsing. There's a lot less block strings (and the documents generally tend to be smaller) in executables so I didn't make that optimisation here.
But I might remove the BlockString variant from Value. The main thing I wanted to optimise was docstrings - there's so many use cases that do not need them, so taking significant time to always process them seemed silly. But Value is a lot more likely to be used (and also wouldn't expect as many of them as blockstrings).
There was a problem hiding this comment.
Ah, good to know, that makes sense.
I am conflicted on whether this should be PartialEq or a method, but since there is only one useful way to compare {types,values}, I went with PartialEq, as well as Eq for types (Value has a Float variant).