Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #2836 (closed enhancement: wontfix)

Opened 3 years ago

Last modified 3 years ago

[PATCH] Move valid options into a class attribute

Reported by: Lucas Carlson <lucas@rufy.com> Assigned to: David
Priority: normal Milestone: 1.1
Component: ActiveRecord Version: 0.14.3
Severity: normal Keywords:
Cc: lucas@rufy.com

Description

To facilitate hacking ActiveRecord, it is better to have validate_find_options look at a mutable class variable instead of hardcoding the valid_find_options.

Attachments

valid_options.diff (1.0 kB) - added by Lucas Carlson <lucas@rufy.com> on 11/11/05 19:50:45.
valid_options_r2.diff (1.0 kB) - added by Lucas Carlson <lucas@rufy.com> on 11/12/05 18:17:36.
A little better abstraction and variable naming

Change History

11/11/05 19:50:45 changed by Lucas Carlson <lucas@rufy.com>

  • attachment valid_options.diff added.

11/12/05 13:09:03 changed by david

  • status changed from new to closed.
  • resolution set to wontfix.

valid_options is a very generic term to use for something that binds only to find options. Also, I believe that this restrictions encourage good design: Don't overwrite or add find options, but use delegation instead. So you would make your custom finder be something like "find_with_cookies(:cookies => "yummi", :conditions => "X")" instead of adding :cookies directly to find.

11/12/05 17:38:54 changed by Lucas Carlson <lucas@rufy.com>

  • status changed from closed to reopened.
  • resolution deleted.

I have an application where I need to override construct_finder_sql, even though it is not the recommended way of doing it. I need to be able to have my objects be flexible enough to do whatever I want with them.

Sure it shouldn't be _easy_ for me to play with my objects and I should be very careful when I do it, but should it really be so ugly for me to do so? You really want me to feel like an outlaw because I have a use-case that hadn't been considered or approved yet? This doesn't seem like the Ruby way to me where things are open, flexible, and maleable. Please give it another thought.

It is only a small change to make things slightly more flexible for hackers that don't always follow the rules no matter what people tell them. Isn't that how Ruby, and Rails for that matter, got started in the first place?

11/12/05 17:49:50 changed by Lucas Carlson <lucas@rufy.com>

Let me also point out that currently, the only way you can add options to the array is by overriding the validate_find_options method. You can't call super and you can't use alias so that you can safely do this and if the method changes in the future, the hack breaks the app. If the array is a class variable, anything can happen to the method and I can still add valid find options.

11/12/05 18:02:25 changed by Lucas Carlson <lucas@rufy.com>

May I also point out that good design dictates that you shouldn't override standard library methods, but nothing in Ruby prevents you from doing so and that is what makes Ruby so powerful the few times that you need to break good design. Shouldn't you discourage people from overriding certain methods, but use open and flexible design principals to let people flourish with the code and not worry that in order to do any hack, you have to hack 5 other places?

Python's philosophy is there is one right way of doing things. Ruby's is there are multiple right ways of doing things. That is one of the reasons so many of us love Ruby. Why box us in to the one way you think it should be done?

11/12/05 18:08:52 changed by Lucas Carlson <lucas@rufy.com>

So you would make your custom finder be something like "find_with_cookies(:cookies => "yummi", :conditions => "X")" instead of adding :cookies directly to find.

I have an user option that I need to add to every find I use in my whole app to check on the database level if the user has access to the object. My app uses a lot of different kinds of find methods, like the find_by_* methods. Is it really a better idea for me to build find_by_*_and_user as well as count_by_user and others? Seems like the wrong direction to go to me.

11/12/05 18:11:29 changed by anonymous

there is no way any design will meet all needs, might as well make it as malleable as possible, it's not like it's going to screw anyone over if a programmer hacks their own version of Rails and your change only allows it to happen easier, it's not like you are adding your hack to the code base, just the part that makes hacking easier.

11/12/05 18:17:36 changed by Lucas Carlson <lucas@rufy.com>

  • attachment valid_options_r2.diff added.

A little better abstraction and variable naming

11/12/05 21:15:02 changed by bitsweat

  • status changed from reopened to closed.
  • resolution set to wontfix.

This is making a mountain out of a molehill, Lucas. Exposing the valid options has code smell. Just override the method. This is not entirely ideal; that's okay. When new versions are released you'll have bigger things to worry about than keeping that array in sync, considering you're overriding construct_finder_sql.