Closed tommay closed 9 years ago
kenichi
commented
9 years ago hmm, interesting. i certainly stole that directly from sinatra, i have a recollection that there was scope binding issues and UnboundMethod was The Way Around; but i can't remember exactly.
https://github.com/sinatra/sinatra/blob/master/lib/sinatra/base.rb#L1601
tommay
commented
9 years ago I looked through the sinatra source and git history and my best guess for why they use an UnboundMethod is because they need to run a block in their object's context and they needed to pass parameters to it. instance_exec does both of these things (instance_eval doesn't pass parameters) but I believe it was added to ruby in 1.9 and that bit of sinatra was probably coded for 1.8.
Unless/until there's a use case or a failing test that needs UnboundMethod, I'm for keeping angelo simple and non-baffling. It certainly baffled me. And if a time does come when UnboundMethod is required, we'll know why and can drop in some comments.
On Thu, Jan 29, 2015 at 11:56 AM, kenichi nakamura <notifications@github.com
wrote:
hmm, interesting. i certainly stole that directly from sinatra, i have a recollection that there was scope binding issues and UnboundMethod was The Way Around; but i can't remember exactly.
— Reply to this email directly or view it on GitHub https://github.com/kenichi/angelo/pull/38#issuecomment-72092767.
kenichi
commented
9 years ago i agree. unboundmethod seems to be unnecessary black magic at this point.
I was trying to figure out what compile! was good for. It converts a block to an UnboundMethod that can be bound to an Angelo::Base instance and called. But why bother. It seems more ruby to keep it as a block and call it with instance_eval, or instance_exec if it needs arguments.
This PR is strictly a refactoring with three somewhat related commits. Take them or leave them.
Sorry to be kind of hyperactive. I've actually got any number of small tweaks to suggest that I've come across while going through the code.