This version of WWWTC is dedicated to closures. We can use closures and functional programming in general to create elegant, succinct solutions, but sometimes we can also create subtle bugs.
Exhibit A. The C# developer who wrote this code expected DoSomething to be invoked with the values 0 through 9, but something is terribly wrong.
for (int counter = 0; counter < 10; counter++) { ThreadPool.QueueUserWorkItem( state => DoSomething(counter) ); }
What’s the problem?
A JavaScript developer is working with the following markup.
<div class="header"> <div class="detail">Detail 1 …</div> </div> <div class="header"> <div class="detail">Detail 2 …</div> </div> <!-- and so on-->
When the user clicks inside a detail section, the developer wants to change the background color of all the other header sections. Thinking of one way to achieve this behavior, the developer first tests the jQuery not method.
$(function() { $(".header").each(function() { alert($(".header").not(this).length); return false; }); });
This code correctly alerts the user once and displays the total number of headers – 1.Emboldened with this proof of concept, the developer puts in the required CSS manipulation and embeds the code into a click event handler.
$(function() { $(".header").each(function() { $(".detail", this).click(function() { $(".header").not(this).css("background-color", "red"); }); }); });
Suddenly, things aren’t working and every header section turns red. What went wrong?
Comments
To solve it, add a var that=this; in the third line, and make it
$(".header").not(that).css("background-color", "red");
2)Jquery in most of it's chaining methods makes "this" to point to current element. So, the code not works as expected, because outer this points to a .header, while inner this points to a .detail
The JavaScript problem is caused by a change in context. .not(this) now refers to elements with a .detail class.
For the bonus round, the issue is with the function passed into the detail's click. In that function "this" references the item clicked on and not the parent header element. So, all headers will turn red, as the not operator will not match the correct header element.
The details in IL over here: skepticabin.wordpress.com/.../delegate-magic/
I haven't worked with jquery enough to understand your bonus round.
Value passed to DoSomething will be whatever counter is at the time the queued work item is executed (e.g. 9 if delegate invoked after loop has finished). An additional variable would need to be added within the for loop, which would be captured by the delegate.
Bonus:
Without running it I can't figure out what each of the 'this' variable will contain but I believe the code should be written like this:
$(function() {
$(".header").each(function() {
var thatHeader = this;
$(".detail", thatHeader).click(function() {
var thatDetail = this;
$(".header").not(thatDetail).css("background-color", "red");
});
});
});
Is it that the line:
$(".header").not(this).css("background-color", "red");
... the "this" is referring to the "detail" div, so essentially the line is grabbing and "header" that is not the same as "detail", which is all of them, hence all are changed.
Maybe if the "header" was stored in a var before the "click", then used, something like this ? .....
$(function() {
$(".header").each(function() {
var h = this;
$(".detail", this).click(function() {
$(".header").not(h).css("background-color", "red");
});
});
});
$(".header").not(this.parent()).css("background-color", "red");
Also, on the click you should really remove the css property background-color otherwise as soon as you click on another detail div all headers will end up red.
The Lambda expression captures the *variable* counter and not the value. By the time DoSomething(counter) is called by the threadpool, the value of counter is the last value that was assigned in the loop. Not neccessarily 9 since you could have a context switch in the middle of the loop. If counter is 4 and you have a context switch, all background calls to DoSomething() will have the value 4 for counter.