6
\$\begingroup\$

The code allows you to select an area from the left column and another area from the right column followed by clicking on the Choose button which sends the chosen areas to the server:

<html>
  <head>
    <style>
    body {
      overflow: hidden;
    }

    article.left {
      overflow: hidden;
      float: left;
    }

    article.left section {
      float: left;
    }

    section {
      border: 1px solid black;
      height: 6em;
      margin-right: 1em;
      width: 4em;
    }

    article.right section {
      border: 1px dashed black;
    }

    section.ice {
      transform:rotate(-90deg);
      -moz-transform:rotate(-90deg); 
      -webkit-transform:rotate(-90deg); 
    }

    article.right {
      float: right;
    }

    section.section-selected, section.right-selected {
      border-color: #EEE;
    }

    input.choose {
      display: none;
    }
    </style>
    <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.9.1/jquery.min.js"></script>
    <script>
    $(document).ready(function() {
      $('article.left section').click(function() {
        var was_selected = $(this).hasClass('section-selected');
        $('article.left section').removeClass('section-selected');
        if (!was_selected) {
          $(this).addClass('section-selected');
        }
      });

      $('article.right section').click(function() {
        $(this).toggleClass('right-selected');
        if ($('section.right-selected')) { 
          $(this).children('input.choose').toggle();
        }
      });

      $('input.choose').click(function() {
        var section = $('section.section-selected');
        if (section.length) {
          console.log(section.attr('section-id') + ' ' + $(this).attr('location-type'));
          console.log($(this).parents('article').attr('article-id'));
        }
        else {
          console.log('none selected');
        }
      });
    });
    </script>
  </head>
  <body>
    <article article-id="L" class="left">
      <section section-id="A">A</section>
      <section section-id="B">B</section>
    </article>

    <article article-id="R" class="right">
      <section section-id="C"><input type="button" class="choose" location-type="vertical" value="Choose" /></section>
      <section section-id="D" class="horizontal"><input type="button" class="choose" location-type="horizontal" value="Choose" /></section>
    </article>
  </body>
</html>

Here's a link to the jsfiddle http://jsfiddle.net/95WvB/. The code is working fine but I am wondering if the above is what is considered as spaghetti code that so many of those js frameworks like ember or angular are trying to solve. Is it best to use one of those framework to refactor the above code or use backbone?

This code is a sample of a much larger web application which repeats more or less of the same interactions between front and backend.

\$\endgroup\$
5
  • 1
    \$\begingroup\$ Why is your JavaScript, CSS and HTML all on the same file? Other than that, cache selector. I wouldn't use ember or angular for this sort of code since it is very small compared to a real web "application" \$\endgroup\$
    – Benjamin Gruenbaum
    Commented Mar 21, 2013 at 19:59
  • \$\begingroup\$ Just trying to make it simpler for people to test the code quickly, I separate the css and javascript in their own files in dev and production. \$\endgroup\$
    – Thierry Lam
    Commented Mar 21, 2013 at 20:02
  • \$\begingroup\$ "if ($('section.right-selected'))" - does this work properly? Empty arrays evaluate to true in Javascript so I thought "empty" jQuery objects would as well. \$\endgroup\$
    – Krotton
    Commented Mar 26, 2013 at 20:46
  • \$\begingroup\$ $('section.right-selected') evaluates to [] which is false, you can test it in the chrome console. Not sure if other browsers give different output for that. \$\endgroup\$
    – Thierry Lam
    Commented Mar 26, 2013 at 21:03
  • 2
    \$\begingroup\$ A) $('section.right-selected') doesn't evaluate to [], it evaluates to a jQuery object. B) Both [] as well as all objects are considered true in JavaScript. If you want to know if $('section.right-selected') has selected anything, you have to use if ($('section.right-selected').length > 0) {... . \$\endgroup\$
    – RoToRa
    Commented Apr 26, 2013 at 12:45

2 Answers 2

Reset to default
2
\$\begingroup\$

I would do it the following way: As far as I got it, you want to select several Areas (e.g. something like Papersize and landscape or portrait format) and want to keep state of what selected so far.

I did not refactor your whole code, but reengineer it a bit:

<div id="container">    
    <div class="left unselected" id="a">A</div>
    <div class="left unselected" id="b">B</div>
</div>

For the sake of the example I limit my self to these two Areas. You could easily extrapolate to your needs.

First I bound the forEach-function from the array with the following statement:

var forEach=Function.prototype.call.bind([].forEach);

After that I used an object to keep the state for me:

var selectedItems={
    a:false,
    b:false,
    c:false,
    d:false
};

That's what you usually would do with something like a "Model"

(cf. Backbone.Model.extend({}); in http://backbonejs.org/).

But for this usecase, that simple object would do the trick.

Next I would define a toggle function for each group (in my example only for "left"):

var toggleSelectionLeft=function(elem){
    target=$(elem.target);
    target.hasClass("selected")?target.removeClass("selected"):target.addClass("selected"); 
    var id=elem.target.id;
    selectedItems[id]=!selectedItems[id];
};

This togglefunction handles the adaption of the css and sets the state of our object. Usually you would further decouple these tasks, in only triggering an "elementSelected" event, to which thwo functions would subcribe: (1) update the model and (2) adapt the CSS. These are two different tasks. For the sake of this simple example I left them together (So don't do this at home g).

After all your behaviour is declared, you could go on, binding the "click" to the wanted behaviour:

forEach($(".left"), function(elem){
    $(elem).on("click", toggleSelectionLeft);
});

A working example is under: http://jsfiddle.net/Susv4/ For submitting values to the server you could easily evaluate our "model".

\$\endgroup\$
1
\$\begingroup\$

This bit needs refactoring. If you are writing a one time script or throw away code ,and you are absolutely certain it won't be more than throw away code, you needn't worry about refactoring.

But since this is a part of a large application everything needs a name indicating what it is or does, making explicit as much as possible implicit structure etc..

For example:

  $('article.left section').click(function() {
    var was_selected = $(this).hasClass('section-selected');
    $('article.left section').removeClass('section-selected');
    if (!was_selected) {
      $(this).addClass('section-selected');
    }
  });

Can be changed to

function selectAtMostOne(selection, className) {
  selection.click(function() {
    var was_selected = $(this).hasClass(className);
    selection.removeClass(className);
    if (!was_selected) {
      $(this).addClass(className);
    }
  });
}
selectAtMostOne($('article.left section'), 'section-selected');

And if you are doing this selectAtMostOnce [or whatever it was supposed to do] multiple times, you can add it to jquery or do something else to make using it easier, as in: 

$.fn.extend({
    selectAtMostOne: function(options) {
        var defaults = {
            selectedClass : 'selected'
        }
        var options = $.extend(defaults, options);

        var selection = this;
          return selection.click(function() {
            var was_selected = $(this).hasClass(options.selectedClass);
            selection.removeClass(options.selectedClass);
            if (!was_selected) {
              $(this).addClass(options.selectedClass);
            }
          });
}
});

$('article.left section').selectAtMostOne({selectedClass : 'section-selected'});

As an additional benefit, this will remind you to use variables instead of copy-pasted string constants. If you mistype any variable, hopefully your IDE or static-analysis (jslint etc) will tell you that you are using a variable before declaring it, or will have undefined error; which will be easier to debug than the case of mistyped string constants.

\$\endgroup\$
0

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.