HotCocoa / hotcocoa

MacRuby HotCocoa UI library
120 stars 8 forks source link

Too much #eval of strings #6

Open ferrous26 opened 13 years ago

ferrous26 commented 13 years ago

There is entirely too much use of #eval (and close family) of strings in HotCocoa, which leaks memory in MacRuby and is slower than alternatives. In some cases it is inappropriate given the meta-programming hooks available in Ruby 1.9, in other cases we can refactor to get rid of it.

hotcocoa/delegate_builder.rb
57:    eval %{

hotcocoa/graphics/canvas.rb
145:          send(:instance_eval, &block)

hotcocoa/graphics/color.rb
200:      metaclass.instance_eval do

hotcocoa/graphics/path.rb
54:          self.send(:instance_eval, &block)

hotcocoa/kvo_accessors.rb
29:    c.module_eval &b

hotcocoa/mapper.rb
59:    mod.module_eval(&block)
179:          delegate_module.module_eval <<-EOM
189:        delegate_module.module_eval <<-EOM
220:      bindings_module.module_eval <<-EOM

hotcocoa/mapping_methods.rb
107:      @custom_methods.module_eval(&block)

hotcocoa/mappings/appkit/color.rb
4:      color = eval("NSColor.#{options.delete(:name)}Color")

hotcocoa/mappings/appkit/font.rb
34:        font = eval("NSFont.#{method}(#{options.delete(key)})")

hotcocoa/mvc.rb
140:    HotCocoaController.class_eval %{
ferrous26 commented 13 years ago

Took care of lib/hotcocoa/mapper.rb:220

ferrous26 commented 13 years ago

Took care of hotcocoa/mappings/appkit/color.rb:4

ferrous26 commented 13 years ago

Took care of hotcocoa/mappings/appkit/font.rb:34

ferrous26 commented 13 years ago

I think for hotcocoa/delegate_builder.rb:57 we can only make the code inside of eval smaller, but because of the delegation mechanics we cannot completely remove it.

ferrous26 commented 13 years ago

Took care of hotcocoa/mapping_methods:107

isaac commented 13 years ago

Hey, I'm curious if you're OK with methods like Module#define_method which can use module_eval under the covers. Or is it just evaling strings that you want to minimize in HotCocoa?

ferrous26 commented 13 years ago

@isaac yes, it is the eval'ing of strings that I want to minimize, Module#define_method is, I think, the suitable replacement in most cases, especially where #module_eval is being called just to define a method.

ferrous26 commented 13 years ago

I've adjusted the bug to better reflect what I meant. Thanks for pointing that out, @isaac

ferrous26 commented 13 years ago

mapper.rb:59 is not changeable

ferrous26 commented 13 years ago

mapper.rb:179 and mapper.rb:189 are now taken care of

mapper.rb:179 has an odd looking solution, it is a workaround to a small issue I was having with MacRuby, not sure if it is a MacRuby bug or not

ferrous26 commented 13 years ago

The only cases left are:

delegate_builder.rb:57

This one is kind of important since it is used by every delegate that is set up using Hot Cocoa style delegation. Though, we can only reduce the amount of code that needs to be evaluated as a string.

mvc.rb:140

Not too big a deal right now. I haven't even gotten around to really looking at the file yet and it has to be explicitly loaded. No examples use this, and it had a syntax issue that was not discovered until we tried compiling the file.

ferrous26 commented 13 years ago

The reason for the workaround has been discovered to be a MacRuby bug. See MacRuby Ticket #1371.