8 ms·

Abstract Methods and NotImplementedError in Ruby

52 points by nithinbekal 2y ago

MarceColl 2y ago
I think that NotImplementedError doesn't get rescued is a feature, not a bug. It means you are calling a class that has a missing part of the interface, and that's usually programmer error. For some cases where you can partially implement the spec, it may make sense to go this way, but for most abstract classes I think the NotImplementedError is still a good idea.
- lamontcg 2y ago
  Yeah, an unimplemented abstract method being called shouldn't really ever happen at runtime and should be some form of "Internal Error" and should aggressively try to blow up the program. I think this blog post just convinced me using it is correct. The concrete class that leaves it unimplemented should raise something else if they intend to let consumers catch it with a generic rescue clause (which is probably a code smell itself).
- looopTools 2y ago
  I totally agree with this point of view
- yawboakye 2y ago
  that’s an accidental feature given that ruby’s own expectation, going by the description in the blog post, is that a required feature is missing on current platform. perhaps it’s badly named. because it leads to this faux ami. speaking as someone who has raised quite a lot of them myself, and i think that’s what sorbet raises as well.
- e12e 2y ago
  It's probably a bigger problem that afaik #respond_to? will return true for an "abstract" method. Which is a bit awkward.
Traubenfuchs 2y ago
How about... and please sit down for this one... How about adding an "abstract" keyword for methods and refusing to compile with an abstract method from a parent class not being implemented in a non abstract child class?
- raincole 2y ago
  Or, you know, people use different programming languages for reasons and there isn't need to unify all the languages as one?
- rezonant 2y ago
  It's Ruby, there's no compilation. It's also common to not load all files of an application's codebase at startup, in particular this is the default setting for Rails in development mode.
  - lloeki 2y ago
    There is. CRuby code gets compiled to ISeq (VM bytecode) upon `load` / `require` / `eval`. JRuby does similar things (IIRC using InvokeDynamic). The difference (and that does not even need to involve Rails) is that these three - and thus compilation - happen at runtime, anytime, always. The fact that a typical "main" file follows the "require first, then define modules and classes, then execute" creates a mental illusion of order; the fact that generally dependencies also follow this propagates that illusion across the board. Therefore at the ruby source level you can only have runtime checks (and failure via exceptions), unless you bring in another static analysis tool such as Sorbet or RBS+Steep, which is an actual solution to enforce interface contracts without risking blowing up an app at runtime. That stays true even if there were a hypothetical "abstract" Ruby-level keyword. The only other solution without such tools is to have perfect (runtime) coverage via tests. Note that mruby takes a fundamentally different approach here, and there's no require nor load.
    - byroot 2y ago
      > thus compilation - happen at runtime, anytime, always. To be a bit pedantic, you can compile Ruby code with `RubyVM::InstructionSequence.compile`, and then dump and load it as a String. That's one of the things Bootsnap does to speedup boot time. And when you do that, there's no compilation at runtime. But that doesn't change anything about OP's suggestion, it's still impossible to know if an interface will ever be implemented. https://docs.ruby-lang.org/en/3.3/RubyVM/InstructionSequence.html https://docs.ruby-lang.org/en/3.3/RubyVM/InstructionSequence...
thomascountz 2y ago
Ongoing related discussion: https://bugs.ruby-lang.org/issues/18915 https://bugs.ruby-lang.org/issues/18915
irjustin 2y ago
Wow I didn't know this. I and thusly our team uses `NotImplementedError` all over the place and I know I picked up that pattern from a rubygem somewhere along the line (predates copilot or generative code by years). Even though I'm technically using it incorrectly, I don't want it to inherit `StandardError` and thus blows up. Because, I want it to blow up in the worst way possible. The handling engineer knows this is serious, and it if made it all the way to production, then our specs are lacking so bad that we need to have a discussion. That's a feature to me.
ngcazz 2y ago
I have always been unconvinced of the value of doing Java-style abstract classes to assert a certain API in Ruby considering it's duck-typed. Raising an error when at runtime you call a nonexistent method is native Ruby behavior, so why write methods that say "I don't exist"? If doing this added in any build-time guarantees that you have to implement that method they'd be great, but as used they're just additional code for one to be aware of, slowing everyone down at code review and shifting the mental model away from the messages that are being passed. If you need to document a duck type there probably are better ways to do it, and Ruby also provides inheritance hooks that could conceivably be used to detect the existence of methods at load time, but I've never seen that being used in the relatively numerous private code bases I've come across. (Sorry for the numerous edits, the point became clearer after I posted this)
- byroot 2y ago
  > Raising an error when at runtime you call a nonexistent method is native Ruby behavior, so why write methods that say "I don't exist"? Well, first for documentation as you pointed. Second because otherwise the error is a `NoMethodError` which inherits from `StandardError` hence may be casually rescued. > Ruby also provides inheritance hooks that could conceivably be used to detect the existence of methods at load time It's not every practical because the hook is called while the child class is opened, so no method is yet implemented into it. You'd need to collect all the child class and then check they implement all the necessary methods at a later point when you are done loading, which isn't a clearly defined point in a general Ruby program.
- inopinatus 2y ago
  There's a very simple reason. Writing explicit abstract methods ensures that both the parent and its subclasses can still use "method_missing" without having to special-case the landmine¹. > Ruby also provides inheritance hooks that could conceivably be used to detect the existence of methods at load time This misses numerous corner cases e.g. refinements, or the dynamic include/prepend of modules (which may even be generated anonymous modules). One may not write such code often in applications, but this happens routinely in the guts of many frameworks and libraries, and not just Rails. Most attempts to introduce type checking or anything resembling it into Ruby run into the wall of "load time is run time". ___ [1] corollary: consider writing a respond_to? that returns false for the abstract method(s)
yawboakye 2y ago
`UndefinedMethodError` perhaps? that way we can restore the distinction between method declaration (which assigns a name, within some namespace) and method definition/implementation which gives the function its ability.
- nithinbekal 2y ago
  Wouldn't that be too similar to NoMethodError? I recently came across the idiom in Smalltalk, where you would call subclassResponsibility: someMethod: self subclassResponsibility I've suggested the name in the ruby bug tracker issue here: https://bugs.ruby-lang.org/issues/18915 https://bugs.ruby-lang.org/issues/18915
  - yawboakye 2y ago
    linguistically speaking, no method means the name isn’t even known, which isn’t the case here. the name exists and is known (after a successful declaration). what’s missing is the actual definition. if we lean heavily into language, perhaps only undefined method will do.
brandall10 2y ago
Back in the day I recall that NotImplementedError was used for metaprogramming dynamic methods - in particular the ActiveRecord helpers that translated to semi-natural language based on fields in the db... it would try to create the method on the fly if the fields existed and the convention was adhered to, otherwise it would rethrow. I don't believe those are supported anymore.
- byroot 2y ago
  You are thinking of method_missing. It’s an entirely different thing.
  - brandall10 2y ago
    Ah, thank you for the correction. It’s been about 8 years since I last worked with ruby in a regular capacity.

52 points by nithinbekal 2y ago

I think that NotImplementedError doesn't get rescued is a feature, not a bug. It means you are calling a class that has a missing part of the interface, and that's usually programmer error. For some cases where you can partially implement the spec, it may make sense to go this way, but for most abstract classes I think the NotImplementedError is still a good idea.

lamontcg 2y ago
Yeah, an unimplemented abstract method being called shouldn't really ever happen at runtime and should be some form of "Internal Error" and should aggressively try to blow up the program. I think this blog post just convinced me using it is correct. The concrete class that leaves it unimplemented should raise something else if they intend to let consumers catch it with a generic rescue clause (which is probably a code smell itself).
looopTools 2y ago
I totally agree with this point of view
yawboakye 2y ago
that’s an accidental feature given that ruby’s own expectation, going by the description in the blog post, is that a required feature is missing on current platform. perhaps it’s badly named. because it leads to this faux ami. speaking as someone who has raised quite a lot of them myself, and i think that’s what sorbet raises as well.
e12e 2y ago
It's probably a bigger problem that afaik #respond_to? will return true for an "abstract" method. Which is a bit awkward.

Traubenfuchs 2y ago

How about... and please sit down for this one... How about adding an "abstract" keyword for methods and refusing to compile with an abstract method from a parent class not being implemented in a non abstract child class?

raincole 2y ago
Or, you know, people use different programming languages for reasons and there isn't need to unify all the languages as one?
rezonant 2y ago
It's Ruby, there's no compilation. It's also common to not load all files of an application's codebase at startup, in particular this is the default setting for Rails in development mode.
- lloeki 2y ago
  There is. CRuby code gets compiled to ISeq (VM bytecode) upon `load` / `require` / `eval`. JRuby does similar things (IIRC using InvokeDynamic). The difference (and that does not even need to involve Rails) is that these three - and thus compilation - happen at runtime, anytime, always. The fact that a typical "main" file follows the "require first, then define modules and classes, then execute" creates a mental illusion of order; the fact that generally dependencies also follow this propagates that illusion across the board. Therefore at the ruby source level you can only have runtime checks (and failure via exceptions), unless you bring in another static analysis tool such as Sorbet or RBS+Steep, which is an actual solution to enforce interface contracts without risking blowing up an app at runtime. That stays true even if there were a hypothetical "abstract" Ruby-level keyword. The only other solution without such tools is to have perfect (runtime) coverage via tests. Note that mruby takes a fundamentally different approach here, and there's no require nor load.
  - byroot 2y ago
    > thus compilation - happen at runtime, anytime, always. To be a bit pedantic, you can compile Ruby code with `RubyVM::InstructionSequence.compile`, and then dump and load it as a String. That's one of the things Bootsnap does to speedup boot time. And when you do that, there's no compilation at runtime. But that doesn't change anything about OP's suggestion, it's still impossible to know if an interface will ever be implemented. https://docs.ruby-lang.org/en/3.3/RubyVM/InstructionSequence.html https://docs.ruby-lang.org/en/3.3/RubyVM/InstructionSequence...

thomascountz 2y ago

Ongoing related discussion: https://bugs.ruby-lang.org/issues/18915 https://bugs.ruby-lang.org/issues/18915

irjustin 2y ago

Wow I didn't know this. I and thusly our team uses `NotImplementedError` all over the place and I know I picked up that pattern from a rubygem somewhere along the line (predates copilot or generative code by years). Even though I'm technically using it incorrectly, I don't want it to inherit `StandardError` and thus blows up. Because, I want it to blow up in the worst way possible. The handling engineer knows this is serious, and it if made it all the way to production, then our specs are lacking so bad that we need to have a discussion. That's a feature to me.

ngcazz 2y ago

I have always been unconvinced of the value of doing Java-style abstract classes to assert a certain API in Ruby considering it's duck-typed. Raising an error when at runtime you call a nonexistent method is native Ruby behavior, so why write methods that say "I don't exist"? If doing this added in any build-time guarantees that you have to implement that method they'd be great, but as used they're just additional code for one to be aware of, slowing everyone down at code review and shifting the mental model away from the messages that are being passed. If you need to document a duck type there probably are better ways to do it, and Ruby also provides inheritance hooks that could conceivably be used to detect the existence of methods at load time, but I've never seen that being used in the relatively numerous private code bases I've come across. (Sorry for the numerous edits, the point became clearer after I posted this)

byroot 2y ago
> Raising an error when at runtime you call a nonexistent method is native Ruby behavior, so why write methods that say "I don't exist"? Well, first for documentation as you pointed. Second because otherwise the error is a `NoMethodError` which inherits from `StandardError` hence may be casually rescued. > Ruby also provides inheritance hooks that could conceivably be used to detect the existence of methods at load time It's not every practical because the hook is called while the child class is opened, so no method is yet implemented into it. You'd need to collect all the child class and then check they implement all the necessary methods at a later point when you are done loading, which isn't a clearly defined point in a general Ruby program.
inopinatus 2y ago
There's a very simple reason. Writing explicit abstract methods ensures that both the parent and its subclasses can still use "method_missing" without having to special-case the landmine¹. > Ruby also provides inheritance hooks that could conceivably be used to detect the existence of methods at load time This misses numerous corner cases e.g. refinements, or the dynamic include/prepend of modules (which may even be generated anonymous modules). One may not write such code often in applications, but this happens routinely in the guts of many frameworks and libraries, and not just Rails. Most attempts to introduce type checking or anything resembling it into Ruby run into the wall of "load time is run time". ___ [1] corollary: consider writing a respond_to? that returns false for the abstract method(s)

yawboakye 2y ago

`UndefinedMethodError` perhaps? that way we can restore the distinction between method declaration (which assigns a name, within some namespace) and method definition/implementation which gives the function its ability.

nithinbekal 2y ago
Wouldn't that be too similar to NoMethodError? I recently came across the idiom in Smalltalk, where you would call subclassResponsibility: someMethod: self subclassResponsibility I've suggested the name in the ruby bug tracker issue here: https://bugs.ruby-lang.org/issues/18915 https://bugs.ruby-lang.org/issues/18915
- yawboakye 2y ago
  linguistically speaking, no method means the name isn’t even known, which isn’t the case here. the name exists and is known (after a successful declaration). what’s missing is the actual definition. if we lean heavily into language, perhaps only undefined method will do.

brandall10 2y ago

Back in the day I recall that NotImplementedError was used for metaprogramming dynamic methods - in particular the ActiveRecord helpers that translated to semi-natural language based on fields in the db... it would try to create the method on the fly if the fields existed and the convention was adhered to, otherwise it would rethrow. I don't believe those are supported anymore.

byroot 2y ago
You are thinking of method_missing. It’s an entirely different thing.
- brandall10 2y ago
  Ah, thank you for the correction. It’s been about 8 years since I last worked with ruby in a regular capacity.