Remove 'popover' parameter from displayer/hider as it's redundant.

[nohros]

What is wrong is not the parameter, but the use of the $pop over variable inside hider/displayer.

[noordawod]

It's redundant, @nohros, as both the functions and the parameter ($popover) are declared inside the same JS scope. It's pointless to send a parameter to a function when that function has access to it.

[nohros]

It's redundant, but its create a closure for no reason and that should be avoided, if possible, for performance reasons.

To make it more clear:

https://developers.google.com/speed/articles/optimizing-javascript

http://blogs.msdn.com/b/ie/archive/2007/01/04/ie-jscript-performance-recommendations-part-3-javascript-code-inefficiencies.aspx

[noordawod]

I perfectly know what closures are (20+ years experience). However, I am actually fixing YOUR closures and not the other way around :)

Do me a favor and do the following:

Look at both these objects (displayer_ and hider_) and notice how many variables they're accessing which are not in their scope (I'll help you: attrs, scope, options, elm, placement_, align_, $triangle, insideClickHandler, outsideClickHandler and buttonClickHandler.) Quite a bunch, ehh?

In addition, in the current implementation, hider_.hide() accesses both $popover and popover, one is in the outer scope and the later is a function parameter. In addition, although displayer_.display() accepts popover as its first parameter, it actually never uses it -- it uses $popover from outer scope. I'm guessing these two are typos, but you get the idea: I'm trying to fix your code.

So, you reckon that one more variable will cause the library to slow down because of out-of-scope access? I don't think so, to be honest.

If you'd like to make these two objects truly separable from each popover instance, you ought to place them OUTSIDE the link function and then pass parameters to them. Or, create a standard constructor-based approach where each popover instance maintains its own scope and variables. Any of these alternative methods would produce a better result, in my humble opinion.

[nohros]

Yes, I agree with you. displayer and hider objects are created in a wrong way, I think. The alternative methods that you have proposed is the path to follow.

I want to apologize if the latter post was impolite, that was not my intention. You have much more experience in javascript than I have and I appreciate your contribution for the project.

Thanks

[noordawod]

You're most welcome, @nohros. I appreciate your merge and looking forward for more cooperation, I like your work :)

[noordawod]

Another thing: If you're using any kind of IDE, @nohros, you should've been able to find these little misbehavings quite easily. But if you're not using, I guarantee that you'll become a MUCH better developer by using one -- personally, I use NetBeans.

[noordawod]

And one final tip @nohros: I really like developers to be better, so I invested some moments to demonstrate just few of the features of an IDE and prepared a screenshot of nsPopover's as it appears in my NetBeans IDE:

http://i.imgur.com/EBpqai5.png

[nohros]

I use WebStorm, but the actual code is a little different from the original. The displayer and part of the hider code was implemented by someone else and needs to be revised.

[noordawod]

Never tried it myself, but maybe I should -- just for the sake of trying out new editors.