2
\$\begingroup\$

I'm currently working on the library BigPicture.js with live demo here.

There surely are lots of little improvements possible in the main .js file (< 300 lines of code), here are some little questions :

  1. Is it common to use something like :

    (function() {
  // code here
} ) ();

    to launch the code and to be able that return; will work ? Or are there more common schemes ?

  2. Should I place this at the end of the <body> in the mail .html file or in the <head> part? Should I launch this code on something like DOMContentLoaded ?

  3. When zooming to much (using the zoom feature I provide with mousewheel or double-click or PGUP/DOWN etc.) with Chrome (for example when a single letter uses full browser height, i.e. the text is really big), then panning (with CLICK+DRAG ) is really slow. It is not true at all on Firefox, not on IE. What could be the reason (and solution) for this slowness with Chrome only?

  4. Are there some other obvious improvements? I taught myself JavaScript (but with help of SO questions/answers of course), so there are surely some bad coding habits in my code, right?

Code for panning (a little bit shortened, see the project's page for real code)

  dragging = false;

  bpContainer.onmousedown = function(e) {
    dragging = true;
    previous_mouse = {x: e.pageX, y: e.pageY};
  }

  window.onmouseup = function() {
    dragging = false;
  }

  bpContainer.ondragstart = function(e) {
    e.preventDefault();
  }

  bpContainer.onmousemove = function(e) {
    if (dragging) {
      bp.style.transitionDuration = "0s";
      bp.x += e.pageX - previous_mouse.x;
      bp.y += e.pageY - previous_mouse.y;
      bp.style.left = bp.x + 'px'; 
      bp.style.top = bp.y + 'px';
      current.x -= (e.pageX - previous_mouse.x) * current.zoom;
      current.y -= (e.pageY - previous_mouse.y) * current.zoom;
      previous_mouse = {x: e.pageX, y: e.pageY};
    }
  }
\$\endgroup\$
1

2 Answers 2

Reset to default
1
\$\begingroup\$

From a once over:

  • I probably would have put all the code, that little snippet gives us very little to work with
  • dragging is declared with var and pollutes the global namespace, that is a cardinal sin in JS development
  • Use lowerCamelCase so previous_mouse -> previousMouse or rather previousMousePosition or previousPosition
  • Consider using addEventListener instead of assigning straight to for example bpContainer.onmousedown
\$\endgroup\$
1
  • \$\begingroup\$ Ok for dragging, I'll put it into var. Just for personal culture why is dragging = cardinal sine? Isn't it sinc ? About the last point, why is addEventListener better than bpContainer.onmousedown? If you have a few more seconds to look, here is the (short) full code : github.com/josephernest/bigpicture.js/blob/master/bigpicture.js \$\endgroup\$
    – Basj
    Commented Dec 5, 2014 at 15:25
1
\$\begingroup\$
  1. I've used that before in my code. I find it is often used in libraries as an "initializer" that sets up the code and DOM for the library rather than forcing the user of the library to have to call an initialization function.
  2. It depends. If you need to interact with the DOM right away(as in, find elements, change properties, or set element events), the code has to be at the bottom of the body. If it's just function declarations and definitions, you can keep your code in the head
  3. Is this a question?
  4. There is nothing that really pops out. You have very nice spacing in your code(everything isn't bunched together) and your formatting is perfect(in my opinion).
\$\endgroup\$
2
  • \$\begingroup\$ Thanks @John for your comments. I updated 3. as a question. Do you have an idea for that? \$\endgroup\$
    – Basj
    Commented Dec 5, 2014 at 11:31
  • \$\begingroup\$ @Basj I don't know much about it myself(as I have not really noticed it), but different browsers are going to use your memory in different ways. I know Chrome retains speed even when multiple tabs are open, but can freeze up in other areas(such as too many XHRs). I think it really all depends on how the browser is using the memory at that time and how your users are using the browser at the current time. \$\endgroup\$
    – SirPython
    Commented Dec 5, 2014 at 20:22

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.