Window-scrolling controller

I generally feel that the pattern I use for development is kind of wrong. I start with simple functionality, then when I realize that some pieces can be reused I usually start to write "scope root level" functions in order to be more DRY. I usually place these functions at the bottom of my "scope". Would these functions (changeColumn, in this case) better be methods of main object? Would this script better be organized in a more object-oriented way? Are there major problems with my coding style?

Unfortunately, I work directly with back-end programmers who don't give a damn about JS and so I am left with myself at this. I am trying to look at “rock stars”, but they mainly write tools, not simple interaction functionality, so for this kind of scripts I rarely get to see examples of really good code to learn from.

```
/** Depends on:
* - jQuery (tested with 1.5.1)
* http://jquery.com/
* - jQuery scrolTo (tested with 1.4.2)
* http://flesler.blogspot.com/2009/05/jqueryscrollto-142-released.html
**/

// Main obj
omami = {
init: function() {
// scrollable area
var content = $('#content').css('overflow-x', 'scroll'),
// flag identifying scroll event
didScroll,
// flag identifying if scroll event
// is caused by mouse / trackpad etc.
// i.e. user action
userScroll,
// flag identifying if scroll event
// is caused by script action
scriptScroll,
// content sections
cols = content.find('.column'),
// hash table to store initial columns x-axis positions
positionsMap = {},
currentColHash,
// Checks if first argument is bigger than second
// and smaller than third
isInRange = function(num, a, b) {
return (num > a && num < b);
};

// store initial columns positions
cols.each(function() {
var col = $(this);
positionsMap[col.attr('id')] = col.position().left;
});

// do

That's a bit to much code for me to go through in one go but I'll take a stab at one part:

// for each user initiated scroll event 
// poll current section
setInterval(function() {
  if ( didScroll && !scriptScroll ) {
    didScroll = false;

    var curScroll = content.scrollLeft(), colID;

    // find what column is selected
    for ( colID in positionsMap ) {
      // safe sex
      if ( {}.hasOwnProperty.call(positionsMap, colID) ) {
        // we compare current left scroll of container element
        // with initial positions of columns
        if ( isInRange(curScroll, positionsMap[colID] - 150, positionsMap[colID] + 410) ) {
          // cut "col-" from column ID
          currentColHash = colID.substring(4);

          // if current col isn't already selected
          if (location.hash.indexOf(currentColHash) == '-1') {
            // indicate user action
            userScroll = true;
            // highlight current column in the address bar
            location.hash = currentColHash;
          }
        }
      }
    }
  }
}, 250);

I'd write like this:

// for each user initiated scroll event 
// poll current section
setInterval(function() {
  if ( !didScroll || scriptScroll ) return;
  didScroll = false;
  var curScroll = content.scrollLeft(), colID;

    // find what column is selected
    for ( colID in positionsMap ) {
      // safe sex
      if ( !{}.hasOwnProperty.call(positionsMap, colID) ) continue;
      // we compare current left scroll of container element
      // with initial positions of columns
      if ( !isInRange(curScroll, positionsMap[colID] - 150, positionsMap[colID] + 410) )
        continue;
      // cut "col-" from column ID
      currentColHash = colID.substring(4);

      // if current col isn't already selected
      if (location.hash.indexOf(currentColHash) !== -1) continue;
      // indicate user action
      userScroll = true;
      // highlight current column in the address bar
      location.hash = currentColHash;
    }
  }
}, 250);

-
Use if-guards to save on indentation.

-
Use typechecking operators === and !== instead of == and !=. Weird things happen otherwise: http://wtfjs.com/2010/02/26/implicit-tostring-fun, http://wtfjs.com/2011/02/11/all-your-commas-are-belong-to-Array

-
str.indexOf returns a number, not a string.

I hope this helped a little. :)

StackExchange Code Review Q#1271, answer score: 4