1
\$\begingroup\$

I made this little library for personal use to make javascripting faster and easier. I would like to know if there are any inefficiencies in it (in size and performance) and possibly if there is a feature you think is missing that would be useful.

//PUT ALL CODE AT THE END OF THE PAGE!!!!!! THIS IS UBER IMPORTANT

//document shorthand
doc=document

//querySelectorAll shorthand
function $q(a){return doc.querySelectorAll(a)}

//returns specific element
function $(a,b){return $q(a)[b||0]}

//function to set properties in a chainable manner
Element.prototype.$p=function(a){for(b in a)this[b]=a[b];return this}

//creates a class that hides elements (notice the use of the function above)
$("head").appendChild(doc.createElement("style").$p({"innerHTML":".wqh{display:none}"}))

Element.prototype.$p({
"removeClass":function(a){this.classList.remove(a)},
"addClass":function(a){this.classList.add(a)},
"hide":function(){this.addClass("wqh")},
"show":function(){this.removeClass("wqh")},
//loads content of a file inside the element, example: element.load("random file.txt")
"load":function(a,b,c){c=this;b=new XMLHttpRequest;b.open("GET",a,!0);b.send();b.onreadystatechange=function(){c.innerHTML=b.responseText}}
})

Example of its uses:

$(".test").addClass("red")
$(".test",1).removeClass("big")
$(".test",2).hide()

for(a in b=$q("p"))
b[a].style.fontFamily="serif",
b[a].style.color="blue",
b[a].style.fontSize="20px";

More readable version of the $p() function:

//function to set properties in a chainable manner
Element.prototype.$p = function(proplist){
    var property;
    for(property in proplist){this[property] = proplist[property]}
    return this;
}
\$\endgroup\$
6
  • 1
    \$\begingroup\$ 1) There shouldn't be a requirement to load it at the end of the body. What if you want to use it before then? 2) The whitespace is really ugly, maybe run it through a linter or beautifier. 3) $ is already used by other libraries. It is also a meaningless name for a function. Please choose something else. 4) querySelectorAll is really slow, and missing from old browsers. You can actually get better performance by writing your own selector routine. 5) Adding properties to built-in constructors is a bad practice. See prototype.js. \$\endgroup\$
    – Dagg
    Mar 12, 2012 at 0:00
  • \$\begingroup\$ 1) It is good practice to put code at the end of the page because it runs the code after the dom is ready and before images are loaded 2) it's made to be compact 3) I rarely use any other library so I don't care 4) I doubt that I can make a perfect copy of queyselectorall that isn't like 100k big and allot slower than the original 5) I could bind these functions to $(), but it wouldn't be as powerful. I want my library to be versatile :) \$\endgroup\$ Mar 12, 2012 at 0:17
  • \$\begingroup\$ I thought you might say something like that, that's why I didn't bother to make this an answer. Still: 1) Maybe for your own scripts, not for a library you expect anyone else to use. 2) minify it. 3) If it's only for your own use, okay. But if you're going to make something like this, why not make it generally useful? 4) You don't want a perfect copy, you want tag names, ids, and class names (in any combination). The rest of the stuff leads to bad coding practices. 5) Again, it comes down to whether you're making this only for yourself, or as a generally useful thing. \$\endgroup\$
    – Dagg
    Mar 12, 2012 at 0:26
  • \$\begingroup\$ I'm making it for personal use, I don't expect anyone to use it, and I will only use it if I have a big project to do 4)I really like the flexibility of queryselectorall, if I need a part of code to be fast, I'll just use getelementbyid or something... \$\endgroup\$ Mar 12, 2012 at 0:41
  • \$\begingroup\$ @GGG answers should not be posted as comments. Then you cause huge comment threads to appear... like this one. Comments are for clarifications, remarks, "comments". Not for answering. \$\endgroup\$
    – ANeves
    Mar 12, 2012 at 10:10

5 Answers 5

6
\$\begingroup\$

Problems with it :

  • in the global namespace
  • use of innerHTML
  • no cross-compatibility
  • messing with object prototypes
  • quite useless
\$\endgroup\$
12
  • 2
    \$\begingroup\$ @williammalo doc=document for one. Not only does it put doc in the global scope, there is no var keyword so this will cause an error in strict mode. \$\endgroup\$
    – Dagg
    Mar 12, 2012 at 0:28
  • 1
    \$\begingroup\$ @williammalo , isn't that what gzip is for? \$\endgroup\$
    – tereško
    Mar 12, 2012 at 0:50
  • 2
    \$\begingroup\$ @williammalo "don't trust minifiers" ... You suffer a severe case of paranoia, the solution to not trusting minifiers is writing your own minifier \$\endgroup\$
    – Raynos
    Mar 12, 2012 at 1:53
  • 3
    \$\begingroup\$ There is nothing "efficient" about your code. Compared with properly formatted code it's harder to read, doesn't run faster and it probably won't load noticeably faster when using gzip. \$\endgroup\$
    – RoToRa
    Mar 12, 2012 at 14:58
  • 1
    \$\begingroup\$ @williammalo , if you cannot take criticism, then you should not try to brag about your code. You have turned this into "why are you so mean" discussion. \$\endgroup\$
    – tereško
    Mar 12, 2012 at 15:15
4
\$\begingroup\$

Going off a little on what others have mentioned, I have a couple of main criticisms:

  1. Manually minified -- it's really difficult to get an idea of what you're really trying to do here. I understand that you don't trust minifiers, but the reason people use those instead of always just writing small code by hand is so that they (and other developers) can make sense of the code when they're working on it, and yet still provide an optimized version for production.
  2. The use of doc as your shorthand -- general concerns about global variables aside, that's just a bad name for a library shortcut variable. What if someone using your library needs a variable named doc, to represent something the user uploads (for example)? Sure they can sort it all out using correct scoping rules, but it'd be too easy for them to forget about it, maybe put their own variable in the global namespace, and then either their code breaks or your library breaks. If you create a namespace for your library, then you can isolate doc from the user's code, and you'll be fine.
  3. Modifying existing object prototypes -- really, I could go either way on this. I've used the Prototype library, which did a lot of that, and it was really nice. I'm also aware that it can really confuse developers, and it can even cause problems. It's generally frowned upon by most "best practices" lists I've seen, so while I wouldn't avoid it completely, I'd recommend using it gently when it makes more sense than creating a wrapper. That's probably one of the bigger selling points jQuery offers -- it creates a wrapper object instead of working with the direct JS object prototypes, so code that's expecting the additional functionality can use it, and code that might somehow rely on the original implementation won't know the difference.

I'd really like to see the code in a non-minified, at least somewhat commented/documented format -- especially Element.prototype.$p. That function looks interesting, but I'm having a hard time following along with it as a one-liner with arguments simply named a and b. Especially the for .. in condition -- a in b=a? I've never come across an assignment used like that before. I'm not saying it's wrong by any means, and maybe it's really common and I just missed it in my journeys, but it doesn't seem very intuitive or easily understandable; that would definitely be worth a couple of comments explaining what it does.

Hope this helps -- or at least makes sense even if it doesn't.

\$\endgroup\$
7
  • \$\begingroup\$ Thanks for the constuctive answer. I don't quite understand what you mean about the doc variable, doesnt it have to be global? If it's not global, how could I use it everywhere? Could you elaborate on that... I'll work on making a more readable version and I'll post it later. \$\endgroup\$ Mar 13, 2012 at 18:19
  • \$\begingroup\$ Also, like I said before, I put my code here hoping that people could find ways to make it smaller. My goal is to make it as ubersmall as possible, js1k style :D. (I'm still going to make a readable version tho, don't worry) \$\endgroup\$ Mar 13, 2012 at 18:24
  • \$\begingroup\$ I made a more readable version of the $p() function for you. It is in the op. \$\endgroup\$ Mar 13, 2012 at 18:42
  • \$\begingroup\$ Okay, $p makes much more sense now :) \$\endgroup\$
    – MCory
    Mar 14, 2012 at 18:24
  • \$\begingroup\$ My main point about "doc" wasn't so much that it's global - most libraries use at least some kind of global variable. My suggestion about that is mostly about the variable name. It's too easy for "doc" to clash with someone else's variable name, whereas jQuery (for example) uses the name of the library ("jQuery") or "$" for shorthand. Not a lot of people are going to use "$" as an identifier, and if you include jQuery in your project, then you'll avoid using "jQuery" as an identifier unless you're intentionally trying to screw things up. (Sorry - not used to commenting on here). \$\endgroup\$
    – MCory
    Mar 14, 2012 at 18:32
1
\$\begingroup\$

The only thing that I can see is that b is escaping into the global scope in $p(). However, b doesn't seem to be accessed globally anywhere else in the code you have posted so I only mention it as something to be careful of for future expansion.

You can avoid this without having to use a var statement by including a parameter b to $p and never passing a value for it:

Element.prototype.$p=function(a,b){for(b in a)this[b]=a[b];return this}
\$\endgroup\$
2
  • \$\begingroup\$ Doesnt putting "b" in the "for" loop make it local? \$\endgroup\$ Mar 17, 2012 at 1:31
  • \$\begingroup\$ I ran the code from your question in the Chrome console and then queried the value of b. This is what I got: > b "load" > window['b'] "load" So it seems that b is escaping into the global namespace. \$\endgroup\$
    – pgraham
    Apr 11, 2012 at 18:48
0
\$\begingroup\$

For returning a specific element I would recommend using document.queryselector when no index is provided as it would be more apt.

This might be especially appropriate in the edge case that a very large list could be returned. querySelector could also be optimised by the browser.

For example if($('li')) { ... } to check if a list item is present at all within the page.

\$\endgroup\$
0
\$\begingroup\$

Others have commented on the general idea of this library, I agree. You should only use this for one-man projects.

Still,

function $q(a){return doc.querySelectorAll(a)}

could be simply $q = doc.querySelectorAll;

Also this seems overkill:

//creates a class that hides elements (notice the use of the function above)
$("head").appendChild(doc.createElement("style").$p({"innerHTML":".wqh{display:none}"}))

It just makes much more sense to have this in a css file and give it a proper name like hidden.

Finally, if you are going for short, you might as well use d instead of doc.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.