avdi / naught

A toolkit for building Null Object classes in Ruby
MIT License
1.05k stars 53 forks source link

Composing `black_hole`, `predicates_return` and `impersonate`? #55

Open bensheldon opened 10 years ago

bensheldon commented 10 years ago

I'm running into really weird behavior when trying to compose combinations of impersonate, black_hole, and predicates_return(false) in the same naught builder. Should I not do this?

For example, I'd like the behavior that I can impersonate only methods of a class, that those methods are chainable, and that methods ending in ? return a boolean. I can put together a test case if this is supposed to be supported... or I can propose an small warning to the docs (unless that warning is there and I'm missing it).

bensheldon commented 10 years ago

Just to further describe "weird":

Thinking about it now, I'm impersonating an ActiveRecord object, which probably is only defining certain methods once an instance is initialized.

Thanks for the help (or even the sanity check of "don't do that")

avdi commented 10 years ago

There is always a chance that, in the face of sufficient magic in another library, the answer will be "don't do that". However, if it's possible to make it work without too much special-case code I'd like to. Any chance you could work up a minimal reproduction of the issue? Preferably in the form of a test?

bensheldon commented 10 years ago

Sure, I can put together a test. To avoid pulling in ActiveRecord as a test dependency (cringe), I'll try lightly reproducing in a test class how ActiveRecord implements method_missing and possibly respond_to?. And we can see how far that gets us.

bensheldon commented 10 years ago

So just poking at it a bit, it seems like defining method_missing prevents mimic from raising NoMethod exceptions entirely. Here's the changes I made to the existing spec suite: https://github.com/bensheldon/naught/commit/4587e35ce60543c509a89fdc164007fddef2d3ab

(I can submit it as a pull request for easier diffs... though obviously it's failing at this point.)

avdi commented 10 years ago

Thanks for the test case.

This is an interesting case. Defining #method_missing means that, by definition, we can't accurately mimic the class' full interface, since whether it responds to a given method has been delayed to call time.

That said, it does bring into question whether we want some special treatment around #method_missing for mimic. I'm wondering if it should always filter out #method_missing (and #respond_to?/#respond_to_missing?) from the list of methods to mimic?

Anyone have thoughts? @sferik?

sferik commented 9 years ago

That said, it does bring into question whether we want some special treatment around #method_missing for mimic. I'm wondering if it should always filter out #method_missing (and #respond_to?/#respond_to_missing?) from the list of methods to mimic?

Just reviewing issues and seeing this comment now (over one year later). Anyway, I agree, filtering method_missing out of mimic is probably the right thing to do.