stupidedi
stupidedi copied to clipboard
Values::InvalidEnvelopeVal#definition missing?
Hi hi,
Is the definition
method missing from the Values::InvalidEnvelopeVal
? It seems like AbstractVal
assumes all derived classes implement definition
.
Please let me know if I'm misunderstanding something. If I'm correct that it is missing, I'd be happy to take a swing at a PR.
Thanks for your time!
Yes I think there are a few cases where a method isn't implemented in a "concrete" subclass. It's usually just because the method doesn't have a sensible implementation. Ideally the types would've been designed to avoid this from happening, but I've only been able to keep it to a minimum.
How did you run into it? I would want to see what's going on at the call site if stupidedi itself is calling #definition
on an InvalidElementVal
. The error might just be that it was called in the first place, like the caller incorrectly assumes everything has a definition. It probably makes sense to just return nil
, but hard to say without knowing more.
This didn't actually come up at run time. It was found by a type checker 😁
I would accept a PR that makes it throw NoMethodError
with a message that says InvalidEnvelopeVal
doesn't have a definition.
That's assuming you can change InvalidSegmentVal
to do the same without any breakage (now it returns nil
). If that introduces test failures then I'd be fine with both of them returning nil
.