Skip to main content
Code Review

Return to Question

deleted 68 characters in body
Source Link
Jamal
  • 35.2k
  • 13
  • 134
  • 238

I am not certain if this is too much code to ask to look at.

But generally I 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 (changeColumnchangeColumn, in this case) better be methods of main object? Would this script better be organized in a more object-oriented way? Are Are there major problems with my coding style?

I am not certain if this is too much code to ask to look at.

But generally I 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?

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?

deleted 26 characters in body; edited title; edited tags
Source Link
200_success
  • 145.6k
  • 22
  • 190
  • 479

How would I improve structure of this script? Window-scrolling controller

Thanks for any advice.

How would I improve structure of this script?

Thanks for any advice.

Window-scrolling controller

Tweeted twitter.com/#!/StackCodeReview/status/46811058266054656
Source Link

How would I improve structure of this script?

I am not certain if this is too much code to ask to look at.

But generally I 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.

Thanks for any advice.

/** 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;
 });
 // don't bind complex logic directly on scroll –
 // http://ejohn.org/blog/learning-from-twitter/
 content.bind('scroll', function(e) {
 didScroll = true;
 })
 // 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);
 // Controller
 $(window).bind('hashchange', function() {
 var hash = location.hash;
 if (hash != '' && hash != '#') {
 // cut '#' off
 hash = hash.substring(1);
 
 // don't override user action
 if (!userScroll) {
 // indicate that scroll happens programmatically
 scriptScroll = true;
 // do the scrolling
 content.scrollTo(
 ( content.scrollLeft() + $('#col-' + hash).position().left ) - 20
 , 500, {
 onAfter: function() {
 // done with JavaScript scrolling
 // start polling current section again
 scriptScroll = false;
 }
 });
 }
 userScroll = false;
 } else {
 // support back-button up to empty-hash state
 content.scrollTo(0);
 }
 // on page load, scroll to the proper section
 }).trigger('hashchange');
 // scroll to the next/previous column controls
 // just updating location.hash 
 // controller will take care of taking appropriate action
 $('.column .scroll').bind('click', function(e) {
 e.preventDefault();
 
 var arrow = $(this);
 
 // change the column
 changeColumn({
 direction : arrow.hasClass('right') ? 'right' : 'left', 
 currentColumn : arrow.closest('.column')
 });
 });// .scroll bind('click) fn
 
 
 // handle left and right arrows
 $(window).bind('keyup', function(e) {
 // 37 – left arrow
 // 39 - right arrow
 if (e.keyCode == 37 || e.keyCode == 39) {
 e.preventDefault();
 changeColumn({
 direction: (e.keyCode == 39) ? 'right' : 'left'
 });
 }
 });// window bind('keyup') fn
 // updates location.hash with slug of the column
 // user wants to view, either by scrolling to it, 
 // either by navigating to it with arrows/header nav
 //
 // @param {Object} options object
 // @option {String} direction – determens change direction
 // @option {jQuery} currentColumn – currently selected column
 function changeColumn(opts) {
 
 // defaults
 var settings = {
 direction : 'right',
 currentColumn : (function(){
 // calculate current column from hash if it's selected
 var colHash = location.hash.substring(1);
 // if it's not, we suppose we are at first column
 return (colHash.length != 0) ? $('#col-' + colHash) : cols.first();
 })()
 };
 
 // merge options and defaults
 $.extend(settings, opts);
 // what's the next column?
 var nextColumn = 
 (settings.direction == 'right')
 // scroll right
 ? (settings.currentColumn.next().length != 0) 
 // scroll to the next column
 ? settings.currentColumn.next()
 // scroll to the first column
 : settings.currentColumn.siblings().first()
 // scroll left
 : (settings.currentColumn.prev().length != 0) 
 // scroll to the previous column
 ? settings.currentColumn.prev()
 // scroll to the last column
 : settings.currentColumn.siblings().last();
 // update the hash 
 location.hash = nextColumn.attr('id').substring(4);
 }// fn changeColumn
 } // fn omami.init
};// obj omami
default

AltStyle によって変換されたページ (->オリジナル) /