2
\$\begingroup\$

After I made a change to the html structure for a project, I found myself having to change many jquery selectors in many places. To avoid this problem in the future I decided to make a simple function where I pass an element indicating where I am and a string indicating where I want to go. This function should apply whatever logic is necessary to access the given endpoint. I showed this code to someone and he didn't like it, he said I should use multiple functions or an object. I thought that would make the code too much longer for little to no benefit, but I'm not really sure. The code below is for this tool, which is a tree-based time-tracking app, so for example, "node" refers to a node in the tree. The logic is not exhaustive as I only add code to the function on an as-needed basis.

// Access one particular element from another.
function from(start, end) {
  start = $(start)
  if(start.parent().hasClass('headline')) {
    var node = start.parent().parent()
    if(end == 'node')  return node
    else if(end == 'model')  return get_model(node.attr('model_id'))
  }
  if(end == 'headline')  return start.children('.headline')
  if(end == 'child_list')  return start.children('ol')
  if(end == 'each_child')  return start.find('> ol > .model')
  if(end == 'parent')  return start.parents('.model')
  if(end.indexOf('.') != 0)  end = '.' + end
  if(start.hasClass('headline'))  return $(start.children(end))
  return start.find('> .headline > ' + end)
}
\$\endgroup\$
4
\$\begingroup\$

The answer depends, for me, on many things. Some are:

  • is it a prototype or not? Looks like it is
  • are you in a rush? If so, do whatever works for you IF it's a prototype.
  • how big will the whole project be?

Here's a list of things that bother me:

  1. Function name: from? Might make sense to you but it's not a very good name. It's too...generic. I'd take swissArmyKnifeFromElement over from any day - easy to remember, hard to confuse. If this method is somehow part of an object (like your Tree), then it might be OK for a private function.

  2. The arguments might mean something else too - start and end suggest a range. It's potentially misleading. end can be many things (two, so far): a rule, like 'node' or 'child_list' or a class name. Again, misleading - it will only bring pain.

    function findTreeElements(root, rule, params)
    {
      //implementation
    }
    
    //you could call it like this
    findTreeElements(a,'class',{'name':'someClassName'});
    findTreeElements(a,'headline');
    
  3. The more your project grows, the more complex this function will become - in ways that you cannot foresee :) Maybe that's why your friend was talking about objects and multiple functions. jQuery uses objects everywhere and I think that you should do that too:

    • define a tree/list object
    • define a task/node/whatever object
    • have the tree keep a list of tasks/nodes/etc
    • define the methods for manipulating the tree/node/whatever inside the associated objects

    something like (totally random, possibly related, example):

    var list = tree.findNode(someNodeNameOrIndex).tasks();
    

    It's up to Tree object to manage its list of nodes and implement the findNode method (that parses the DOM whatever way you like).

    It's up to the Node object (returned by Tree's findNode) to manage the list of tasks inside that node (or something similar) and implement the tasks function which would work in a similar way to your current find(elem, 'child_list').

Look at the code written by 'better' javascript developers and learn from them. There are many good projects out there that can teach you a lot. Look for decent sized projects, not 'plugins' or basic snippets; browser extensions are also good candidates.

\$\endgroup\$
  • \$\begingroup\$ Nice critique. A lot better than what I was expecting. I somewhat disagree about point 1. I like short function names cuz they're easy to type and take up little space, and I think the comment provides a sufficient explanation. I think your point about the 'end' argument meaning two different things is a good one -- that could get confusing. I also agree about point 3, but I think that since a more sophisticated structure is not immediately needed, building one now would be a form of premature optimization. Thanks for the thoughts. \$\endgroup\$ – Jesse Aldridge Sep 6 '11 at 10:40
  • \$\begingroup\$ I don't believe in 'though shall not' type laws so yes, some things might not make sense in your case (I'm not trying to imply that I'm right anyway) and might simply slow you down with no other benefits. However, it sounds like you're working (mostly) alone on this project - introduce a team and priorities will change :) I wrote about the short names with other people in mind, mostly; you want your code to be readable not only by you but by others too - if you can avoid confusion, avoid it early. \$\endgroup\$ – bkdc Sep 9 '11 at 5:10
  • \$\begingroup\$ Regarding point 3:I wouldn't call it premature optimization (everyone seems to be afraid of it but it's not that bad, provided you have enough experience and KNOW what you're doing). I might call it over-engineering and it can be bad; in this case, if you want to grow this project beyond the 'prototype' level, I think that you'll need to structure it a bit better and make use of some sort of object oriented approach - otherwise, it can quickly grow into a spaghetti mess that nobody will want to come anywhere near of; separation of concerns / object oriented programming might fix that. \$\endgroup\$ – bkdc Sep 9 '11 at 5:18

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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