ID is far more focused than we knew, apparently

Hi Jannes,

Can’t remember which topic this came up in, but check this repl…

choredoor

No cached element nodes. The ID attribute is definitely in play. Something new in JS that nobody is talking about but which hack even the author of this course must have known, else they wouldn’t have given it away in their code.

With your permission I’d like to invite Maciej to join in our exploration.

3 Likes

The code I used above originates in this thread, and I’ve tweaked it only slightly.

ChoreBot JavaScript -- Need Help

2 Likes

Yes, this is weird, even outside of Codecademy it gives the same unexpected result…

Please do involve Maciej :wink: .

3 Likes

I found this little threat, it does link to another topic which might be worth reading.

2 Likes

The argument against using globals is pretty compelling. Seems we have IE to thank for this non-standard approach. Seems we might need to bring this up with the development team and have this project revised to meet with recommendations.

document.getElementById('id')

document.querySelector('#id')

Eager to see how Maciej views this. I’ll leave the repl as it stands for an example.

3 Likes

Hello Jannes, hello Roy :slight_smile:

Thank you for inviting me to this interesting discussion. I hope that my input will not be disappointing.


Seems we have IE to thank for this non-standard approach.

This is part of a broader discussion. When this feature was shipped IE was a potentat. W3C wasn’t interested in working on specification for JavaScript, ECMA wasn’t sure if JavaScript is a thing etc. Developers community were more concerned about ease of use of the language then with the standardization. And when post-IE web browsers started getting some traction they had to be appealing to the users and to the developers. So leaving this obscure quirk was easier than trying to explain to developers (and users) why their sites are not working in given browser while they still work in the IE.

So yeah, we can thank IE, and also community and vendors of modern browsers.


Ok, back to the topic. Knowing that this feature is in the browsers does not really change much. Browsers do not restrict overwriting global variables, so there is a chance that someone might work twenty years in the industry and never notice this.

This feature might get problematic only in very abstract scenarios. Let’s say that I have element with id="result". And in the JavaScript code I use global variable result, but I forgot to declare it. This might result in a hard to debug error, but:

  1. all linters (even the ones built-in into code editors) would notice this error and would let me know;
  2. if I ever assign some value to the result - problem is gone;
  3. global variables seems obsolete, it’s rather uncommon to use them.

So I would not rant over web browsers because of this weird feature.


But, if in one of the courses or in provided learning materials there is a text that says that we don’t have to declare variables to explicitly store DOM elements or if there is a code that uses this feature then yes, that is a problem, and a serious one. This is not part of the ECMA specification and this is not part of the W3C specification for the JavaScript HTML APIs, so this might get removed from all the browsers in any moment. Plus, there are many properties in the window object and with every month next are added. Building anything on an assumption that this given name will never be used as a name for one of the standard properties of the window object is simply wrong.

@mtf the code in the repl, is it from the course? Sorry, it’s almost 1 AM and I am way too sleepy to get through this wall of text (instructions in this project).

3 Likes

Thanks for joining in, Maciej, and for stirring my memory. I completely forgot about this from way back when. It’s not a new problem, just one that never really got put to bed. That it took me with a start is a sign of my foggy memory, and need for stringency in evolving coding practices.

Yes, and no. The code was adapted to exploit the global variables for testing and demonstration. In the course we do define our cached nodes, doorImageN and startButton, but in the supplied code and instructions we still declare the listeners on the globals. This threw me off, and Jannes, as well, hence this ongoing discussion.

Somewhere is the thread that kicked this off. Will try to find it. Found it, sort of. Still looking for the one referred to in this one…

Off topic

Last year I was involved in a two week long discussion about how this code could be refactored. Surpisingly, the member stayed with it for the duration and got herself a slim and trim program as a result. I’ve let it rest rather than harping about it and tried to see the course model from the learner’s perspective, including taking their POV on problems.

That algorithm example from last week came from my refactored version and cannot be plugged into this code. The transformation from one to the other, at working stages took many steps. It was one of those times when I wanted to push the envelope with the author’s code. Whether I ran out of steam or was just plain satisfied with the transformaton is difficult to say. Could be a bit of both.

Also off topic

https://repl.it/@mtf/Chore-Door

3 Likes

For those astute enough, 2 ** n shows up right away. We have a 1 in 2 ** n chance of winnig, n being the number of doors.

2 Likes

Hey @factoradic! Thx for joining ;).

As @mtf pointed out we were both astonished by the fact that directly calling the id from the DOM without using getElementById was possible. Even more so to find out both the hints in the instructions of the chore door exercise and the Walkthrough video present calling doorN directly.

Again and again we encounter eager learners being confused by this or simply copying the code without realizing it is wrong. However there are some to point this out and ask how it is even possible, examples being:

2 Likes

First of all, @mtf this discussion is simply awesome. This should be part of moderators handbook as an example how to engage users. I truly admire your skills and approach. I know that this is not always working out, members usually are not too eager to get involved with refactoring (it works, why should I change anything?), but I am not sure if any other moderator (or at least I) would be able to show this level of determination. Thank you Roy, I learn a lot from you.


@janneslohmeijer I watched the walktrough video and I checked out these discussions, thank you for providing the context. This is definitely a problem, I am kind of surprised that this wasn’t rejected during the review process at Codecademy (amount of new content might be the reason). I imagine that fixing the instructions should not be too hard. I am not sure how Codecademy will aproach problems in the video, I doubt that this video will be shoot one more time. Nonetheless some action must be taken, the question is how we will approach this.

Code provided by Codecademy is simply wrong, even if it works today in modern browsers. I would probably reach out to Daniel and Alyssa, in hope that they will pass it to the content team, but I don’t want to do anything without finding out what is your approach Roy.

3 Likes

@factoradic, thank you for the tremendous compliment. Coming from someone whom I hold in high regard it is indeed gratifying.

You’re very welcome!

Given that you are esteemed and have a good working relationship with Daniel, I would suggest taking the matter up with him directly, and CC Alyssa. You can loop me in if you like. It’s refreshing to see others take part in matters of this nature.

3 Likes

I contacted Daniel and Alyssa via Slack, mainly to see what other moderators think about that. I will let you know when this will be passed to the content team or if it will be rejected.

I digged a bit more when I was preparing the bug report. It turns out that this feature is currently part of the HTML 5.2 W3C Recommendation -> https://www.w3.org/TR/html52/browsers.html#named-access-on-the-window-object. But if specification says this:

window[name]
Returns the indicated element or collection of elements.
As a general rule, relying on this will lead to brittle code. Which IDs end up mapping to this API can vary over time, as new features are added to the Web platform, for example. Instead of this, use document.getElementById() or document.querySelector() .

for me it is clear that this should be corrected.

3 Likes

This should be now resolved :slight_smile: Now instructions directly ask to use defined global variables, for example:

  1. Inside the script.js file, underneath your global variable, assign doorImage1.onclick to a new, empty arrow function.

As expected, for the time being video will not be corrected.

3 Likes

Thanks for going to bat for other, Maciej. Great result!

They might want to revamp their cheat code (others have used it in the past) in the example finished product (or not).

1 Like

I am not sure, but I think they already fixed that :slight_smile:

From the top of the script.js file:

let door1 = document.getElementById('door1');
let door2 = document.getElementById('door2');
let door3 = document.getElementById('door3');
let startButton = document.getElementById('start');
1 Like

When I last checked it was still the old code, as we can see by your example above. If the code is left like that, those first three lines can be removed and the code will still work since the globals are still exposed.

1 Like

This topic was automatically closed 7 days after the last reply. New replies are no longer allowed.