teaminsight/ns-options — Issue #23: Weird stuff happens when you create options named like Namespace instance_methods
Author: kelredd
Created: 2011-11-22 13:49:11+00:00
State: open

If you specify to create an option with the same name as a method on the namespace class, weird stuff happens. This is b/c creating an option defines a method on the namespace class. Not sure what I'd recommend here - it's an interesting approach/design.

Keeping with the same design, I suppose you should just validate the option name (both getter and setter) against some blacklist, say the namespace classe's instance_methods (including parent Object class instance_methods). This would protect from weird stuff happening but would limit the option names you can use. You could throw an ArgumentError saying is reserved - choose a different option name or something. On 1.8.7, Namespace has the following instance_methods:

```
=> ["method", "taguri=", "inspect", "tap", "clone", "public_methods", "__send__", "instance_variable_defined?", "equal?", "freeze", "to_yaml", "each", "to_yaml_style", "local_methods", "define", "methods", "apply", "to_hash", "dup", "instance_variables", "required_set?", "eql?", "extend", "object_id", "send", "id", "singleton_methods", "hash", "taint", "frozen?", "instance_variable_get", "valid?", "instance_of?", "to_a", "metaclass", "option", "type", "to_enum", "instance_eval", "options", "protected_methods", "method_missing", "metaclass=", "==", "gem", "===", "options=", "instance_variable_set", "kind_of?", "respond_to?", "to_s", "to_yaml_properties", "namespace", "class", "instance_exec", "enum_for", "private_methods", "=~", "tainted?", "__id__", "untaint", "nil?", "taguri", "display", "is_a?"]
```

If you don't want to be so heavy handed (ie who cares if I use 'methods' for an option name, Namespace never uses 'methods' anyway), you should at least do a manual blacklist of things the Namespace class does use ('options', 'namespace', 'apply', 'to_hash', etc). The downside here is you have to maintain it manually

-----------

You could also change your design and instead of defining getter/setter methods for each option, you could piggyback on your method missing handler. This may be too drastic of a change (and really no added benefit over the blacklist approach) so I can't say I'd recommend going this direction.

-----------

Anyhoo - just some observations. @jcredding: thoughts?