Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metaprogramming section #46

Closed
bbatsov opened this issue Sep 28, 2011 · 2 comments
Closed

Metaprogramming section #46

bbatsov opened this issue Sep 28, 2011 · 2 comments

Comments

@bbatsov
Copy link
Collaborator

bbatsov commented Sep 28, 2011

I'm about to introduce a new section devoted to metaprogramming. This issue is the place to share your opinion about good and bad practices that you want to see listed in the new section.

@mrflip
Copy link

mrflip commented Mar 27, 2012

  • The block form of class_eval is preferable to the string-interpolated form.

    • when you use the string-interpolated form, always supply __FILE__ and __LINE__, so that your backtraces make sense:

      class_eval "def use_relative_model_naming?; true; end", __FILE__, __LINE__
    • define_method is preferable to class_eval{ def ... }

  • When using class_eval (or other eval) with string interpolation, add a comment block showing its appearance if interpolated (a practice I learned from the rails code):

    # from activesupport/lib/active_support/core_ext/string/output_safety.rb
    UNSAFE_STRING_METHODS.each do |unsafe_method|
      if 'String'.respond_to?(unsafe_method)
        class_eval <<-EOT, __FILE__, __LINE__ + 1
          def #{unsafe_method}(*args, &block)       # def capitalize(*args, &block)
            to_str.#{unsafe_method}(*args, &block)  #   to_str.capitalize(*args, &block)
          end                                       # end
    
          def #{unsafe_method}!(*args)              # def capitalize!(*args)
            @dirty = true                           #   @dirty = true
            super                                   #   super
          end                                       # end
        EOT
      end
    end
  • avoid using method_missing for metaprogramming. Backtraces become messy; the behavior is not listed in #methods; misspelled method calls might silently work (nukes.luanch_state = false). Consider using delegation, proxy, or define_method instead. If you must use method_missing,

    • be sure to also define respond_to?

    • only catch methods with a well-defined prefix, such as find_by_* -- make your code as assertive as possible.

    • call super at the end of your statement

    • delegate to assertive, non-magical methods:

      # bad
      def method_missing?(meth, *args, &block)
        if /^find_by_(?<prop>.*)/ =~ meth
          # ... lots of code to do a find_by
        else
          super
        end
      end
      
      # good
      def method_missing?(meth, *args, &block)
        if /^find_by_(?<prop>.*)/ =~ meth
          find_by(prop, *args, &block)
        else
          super
        end
      end
      
      # best of all, though, would to define_method as each findable attribute is declared  

@bbatsov
Copy link
Collaborator Author

bbatsov commented Mar 28, 2012

Great suggestions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants