HiveBrain v1.2.0
Get Started
← Back to all entries
snippetjavascriptMinor

Is this spaghetti javascript code? How can it be refactored with a javascript library or framework?

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
thisspaghetticanjavascriptwithrefactoredlibraryhowcodeframework

Problem

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:


  
    
    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;
    }
    
    
    
    $(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');
        }
      });
    });
    
  
  
    
      A
      B
    

    
      
      
    
  


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 sp

Solution

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:

    
    A
    B


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".

Code Snippets

<div id="container">    
    <div class="left unselected" id="a">A</div>
    <div class="left unselected" id="b">B</div>
</div>
var forEach=Function.prototype.call.bind([].forEach);
var selectedItems={
    a:false,
    b:false,
    c:false,
    d:false
};
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];
};
forEach($(".left"), function(elem){
    $(elem).on("click", toggleSelectionLeft);
});

Context

StackExchange Code Review Q#24212, answer score: 2

Revisions (0)

No revisions yet.