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

Consider initialising named captures as local variables #28

Closed
Wardrop opened this issue Jan 16, 2014 · 18 comments
Closed

Consider initialising named captures as local variables #28

Wardrop opened this issue Jan 16, 2014 · 18 comments

Comments

@Wardrop
Copy link
Owner

Wardrop commented Jan 16, 2014

Using Binding#local_variable_set, it should be possible to set named captures as local variables within the block of a route, for example:

get '/:id/:title' do
  p id, title
end
@Wardrop
Copy link
Owner Author

Wardrop commented Jan 19, 2014

While editing the binding of a Proc does not appear to be possible, I've had a discussion on IRC relating to whether anyone would ever want to do this. Injecting local variables into the context of a block AFTER it's been defined sounds bad, I'll grant you that, but does sounding bad mean it actually is bad? I find it's common for people to jump down your throat for suggesting something that sounds bad, without ever adequately justifying why it is bad, even in their own head.

Using #instance_exec to change the context in which a Proc is executed is accepted common practice. People rarely raise an eyebrow to it, as long as it's done in a way that isn't surprising (as per principle of least surprise). Injecting local variables is doing something very similar, but with almost zero side-effects. Proc arguments would not override any local variables defined in the Proc's binding, so the potential for "overwriting" pre-existing variables does not exist. It only has the potential to intercept a potential method call, though parsing an argument, adding parentheses, or fully qualifying the call with self all mitigate this problem. This would only cause a problem in instances where this collision occurs and one is not aware that the named captures are automatically defined as local variables. In this sense, the behaviour can be a little surprising, but no more surprising than #instance_exec for example.

I therefore still don't believe there's anything inherently bad about injecting local variables into the scope of a Proc, as long as it's done sensibly in the same respect as #instance_exec should be used sensibly. The main thing working against local variable injection is that the convenience it provides is minimal. This minor benefit may not outweigh the potential for surprise. #instance_exec on the other hand normally provides more convenience and benefits.

Thinking about it, this idea is mostly unnecessary. If local variables are desired, then using local anonymous captures with Proc arguments achieves the same result. The only difference is that the variable names are not defined inline within the pattern.

get '/*/*' do |id, title|
  p id, title
end

Otherwise, if one prefers named captures, then it's only slightly more verbose to do this:

get '/:id/:title' do |captures|
  p captures[:id], captures[:title]
end

All in all, a good mental exercise that provoked some interesting conversation. It was only ever a consideration anyway.

@Wardrop Wardrop closed this as completed Jan 19, 2014
@Spaceghost
Copy link

It seemed to me that it goes against the somewhat vocal dislike of magic.

@Wardrop
Copy link
Owner Author

Wardrop commented Jan 19, 2014

Correct, though there are different degrees of magic. This is rather simple and obvious magic, but like all magic, it carries the potential for confusion and increases the learning curve.

To be honest, this was more an exercise out of curiosity, seeing whether this would even be possible. It's still an interesting idea.

@Spaceghost
Copy link

I don't really agree, but I doubt I can make any kind of argument that would convince you otherwise. I know I wouldn't approach it in this manner, that's for sure.

@Wardrop
Copy link
Owner Author

Wardrop commented Jan 19, 2014

Which part don't you agree with? That there exists different degrees of magic, or that this kind of local variable injection is still an interesting idea?

@Spaceghost
Copy link

That it's rather simple and obvious, if I were to put myself in the place of someone newer to ruby than I am. An object with methods is simpler and requires less magic. It's still an interesting idea because I like magic, it just sounds a bit contradictory to the very notion you present as a reason why scorched is great.

@Wardrop
Copy link
Owner Author

Wardrop commented Jan 20, 2014

I say it's rather obvious because if you had something like...

get '/:id/:title' do
  fetch_by_id(id)
  # bleh
end

One could fairly easily deduce that the local variable id must exist because it's included as a capture in the pattern.

I think the method in which this is achieved makes no difference. Whether this is injected as a local variable, or by using instance_exec against a singleton where id has been defined as a method, the result is essentially the same. If you're already instance_exec'ing the block as is the case with Scorched (route Proc's are instance_exec'd against the controller instance), then local variable injection remains the only option if you want to implement such an interface.

As long as local variable injection doesn't override any other existing local variable of the same name, this is perhaps cleaner than instance_exec'ing against a singleton. Local variable injection just sounds nasty, but I honestly don't think it is.

Of course, local variable injection is not possible, but if it were, I think it would be a respectable bit of magic. On par with instance_exec, method_missing, and the rest of the family. So beyond just sounding bad, can anyone point out anything that makes local variable injection worse than any of the other magic?

You could argue that local variable injection is semantically superior to instance_exec. Making id available as a method via instance_exec is really an attempt to mimic local variables. Why not just make it a local variable? Not to mention if all you want to do is make a variable/method available within the scope of a Proc without changing self, instance_exec is like doing heart surgery with a hammer.

In summary, if you don't have a problem with instance_exec, you shouldn't have a problem with local variable injection.

@Spaceghost
Copy link

Then I think you and I disagree about the interpretation of your claim that scorched isn't magical. It appears to be simple, and looks simple in use, but it uses magic behind the scenes. http://www.youtube.com/watch?v=YWyCCJ6B2WE

@Wardrop
Copy link
Owner Author

Wardrop commented Jan 20, 2014

I'm not denying it's magic. It absolutely is magic. I just don't think it's a worse kind of magic to any of the other magic that's commonly used in Ruby. As I said though, I never seriously considered adding this to Scorched as the realisation that local variable injection was not possible meant I never got to that stage of consideration. It was an interesting idea, and as I said my primary interest was in seeing if local variable injection was even possible, as if it was, it would perhaps make for a nice alternative to instance_exec in the future, perhaps in other projects.

Don't forget the title of this issue either "Consider initialising named captures as local variables".

@Wardrop
Copy link
Owner Author

Wardrop commented Jan 20, 2014

I should add, my general philosophy with Scorched is to leave as much magic up to the end user to implement. This means making it possible to do such things as local variable injection (if in fact there is a way to do it). Issue #26 relates to this.

Currently, the large action method would make it difficult for anyone to do this in a project that used Scorched. I intend to break up the action method into at least two new methods, action and dispatcher. The dispatcher would be what you'd override to implement anything like local variable injection.

It's a matter of providing the tools for the user rather than deciding on an end solution like Rails does.

@tmilker
Copy link

tmilker commented Jan 20, 2014

The magic that is important to avoid is the stuff that gets surfaced to the user as implicit knowledge. Things that "just work" or "just happen" with or without the user explicitly asking. These begin as helpers or as a convenience to the user but they have an awful tendency to morph into their own monsters.

In this case, the user has to type: "get '/:id/:title' do" which asks, explicitly, for :id and :title to be surfaced to them in a block scope. This is an already established pattern to retrieve those variables. How that actually happens in the framework doesn't matter to the user though, as long as it doesn't have any side effects or caveats. It's not "magic" to the user at that level of code even if it might use the hideous, most evil type of magic from the point of view of a framework developer to implement.

@Wardrop
Copy link
Owner Author

Wardrop commented Jan 20, 2014

@tmilker Yes, very well said. I've had that distinction in my mind the whole time but just didn't know how to word it. Really, there are two points of discussion here. Whether this counts as the kind of magic Scorched is explicitly against, and whether local variable injection is as horrendous as it sounds.

@Spaceghost
Copy link

I feel as if I've been lead to think that scorched was anything special in this regard, when in fact it isn't at all. I appreciate your idea, but reject the way it's advertised.

I'll kindly bow out and leave room for others in this discussion. Sinatra is pretty nice.

@Wardrop
Copy link
Owner Author

Wardrop commented Jan 20, 2014

@Spaceghost I must admit, you confuse me. I'm not adding local variable injection into Scorched. It's not even possible. It's an interesting concept, but even if it were possible, it doesn't mean it would have made it into Scorched. magic in terms of Ruby programming has many interpretations. @tmilker did well to make a distinction.

Scorched wants to be as magic-free as practical as per @tmilker's definition, while also striving to be as DRY and unopinionated as possible. If data can be inferred and used to reduce verbosity and repetition (therefore a general rule of thumb, making programming more enjoyable), then it's worth considering. The main problem with something like variable injection is that it's perhaps too opinionated. Currently in Scorched, you have a choice between assigning each capture to a local variable, assigning a hash to a single local variable, or to not assign any local variables:

get '/*/*' do |id, title|
  puts id
end

get '/:id/:title' do |vars|
  puts vars[:id]
end

get '/:id/:title' do
  puts request.captures[:id]
end

Introducing local variable injection would somewhat dictate to the user how they should access named captures.

@Spaceghost
Copy link

You should probably check out binding_of_caller.

@Spaceghost
Copy link

And mind that I think it would be wrong to use that, but you seem stuck on it, so I'll point you to what you need and again kindly bow out and take my interest to projects I find more interesting.

@Wardrop
Copy link
Owner Author

Wardrop commented Jan 20, 2014

but you seem stuck on it

@Spaceghost Did you read my reply?

@Spaceghost
Copy link

@Wardrop Please stop trying to bring me into this. I have nothing to gain in this discussion and have contributed all I'm interested in contributing.

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

No branches or pull requests

3 participants