What is the right way to monkey patch rails?

Sam Clearman wrote:

Right now the way i do it is i have at the end of my application.rb file

module ActionController::HttpAuthentication::Basic   def authorization     #code here...   end end

Is there a better way of doing it?

That looks like a standard Monkey Patch.

Got unit tests for it?

Phlip wrote:

Sam Clearman wrote:

Right now the way i do it is i have at the end of my application.rb file

module ActionController::HttpAuthentication::Basic   def authorization     #code here...   end end

Is there a better way of doing it?

That looks like a standard Monkey Patch.

Got unit tests for it?

--    Phlip

hehe... nope.

Sam Clearman wrote:

Is there a better way of doing it?

That looks like a standard Monkey Patch.

Got unit tests for it?

hehe... nope.

Ordinarily, the correct way to do anything - especially an MP - is to unit test it.

However, in the case of Basic Authentication, you would need to mock the server... and the authorizer... and so on. The mocks would generally tell your test what they think it wants to hear, and you'd be back to square one.

So maybe the question how to monkey-patch something is isomorphic to the question how you reasonably test it.

Just make the monkey patch as small as possible, and then submit it to the maintainers... C-:

Hi --

Sam Clearman wrote:

Is there a better way of doing it?

That looks like a standard Monkey Patch.

Got unit tests for it?

hehe... nope.

Ordinarily, the correct way to do anything - especially an MP - is to unit test it.

However, in the case of Basic Authentication, you would need to mock the server... and the authorizer... and so on. The mocks would generally tell your test what they think it wants to hear, and you'd be back to square one.

So maybe the question how to monkey-patch something is isomorphic to the question how you reasonably test it.

It's possible to write your model classes inside a view, and then test them, but I still wouldn't recommend it :slight_smile: I'm all for testing, but I don't think that discussing best practices in other terms has been discredited. Also, there are tests for this class already, which could probably be adapted (depending what the new version is doing, of course).

David

David A. Black wrote:

It's possible to write your model classes inside a view, and then test them, but I still wouldn't recommend it :slight_smile: I'm all for testing, but I don't think that discussing best practices in other terms has been discredited. Also, there are tests for this class already, which could probably be adapted (depending what the new version is doing, of course).

You are answering the question "how can I use tests to help bad designs live longer?"

That's a valid question and answer, but it's non-responsive to my point.

The goal, if you _must_ Monkey Patch, is to slightly increase the odds of a clear diagnostic error message when the real library upgrades out from under the patch.

Hi --

David A. Black wrote:

It's possible to write your model classes inside a view, and then test them, but I still wouldn't recommend it :slight_smile: I'm all for testing, but I don't think that discussing best practices in other terms has been discredited. Also, there are tests for this class already, which could probably be adapted (depending what the new version is doing, of course).

You are answering the question "how can I use tests to help bad designs live longer?"

No, neither asking nor answering that question :slight_smile: I just mean that I don't think that "Can you write a 'reasonable' test suite for it?" is the beginning and end of all examination of whether a given practice is a good one. Among other things, test suites are constrained by test tools, the current state of which I admire greatly but which are themselves subject to change and modification. I think one has to take all of this into account. That's all I mean.

That's a valid question and answer, but it's non-responsive to my point.

The goal, if you _must_ Monkey Patch, is to slightly increase the odds of a clear diagnostic error message when the real library upgrades out from under the patch.

Sure, but that's true of all your code. I don't like to stigmatize code that does something to the framework itself, which, like all other code, can be good or bad, well-maintained or not, etc. The adoption of the term "monkey patch" over the last couple of years is, I think, unfortunate, since it does tend to stigmatize practices that in many cases are perfectly reasonable and rational.

In short, I don't literally append "... but you have to do it well and make sure it works" to everything I say about Ruby, Rails, or software in general, but I think it's kind of understood to be implied :slight_smile:

David