Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Here's the full source to a plugin I'm developing, the relevant code I want to refactor is:

function measure($element, $bubble){
    var scrollTop = $window.scrollTop(),
        viewportHeight = $window.height(),
        readableTop = $element.position().top,
        readableHeight = $element.height(),
        readableBottom = readableTop + readableHeight,
        scrollHeight = viewportHeight / readableHeight * viewportHeight,
        progress = (scrollTop - readableTop) / (readableHeight - viewportHeight),
        total = getTotalMinutes($element),
        remaining = Math.ceil(total - (total * progress)),
        distanceProgress = (scrollTop - readableTop) / readableHeight,
        distanceLiteral = readableTop + distanceProgress * readableHeight + viewportHeight / 2 - $bubble.height() / 2,
        distance = Math.max(readableTop, Math.min(distanceLiteral, readableBottom));

    return {
        remaining: remaining,
        progress: progress,
        distance: distance
    };
}

How might I make this calculation more understandable? I'm not terribly good at math, so I got there by trial and error, pretty much.

Now I'd like to refactor into something more readable or at least trim the repetitive nonsense, but I have no clue where to start.

Could you lend me a hand or give me any advice on how to tackle this?

share|improve this question
add comment

1 Answer

Strictly in terms of code readability, you want to break down a function like this into it's core parts. Additionally, to reduce repetitive behavior you could make it more of a prototype and use some object oriented style to get to your result without a block of math.

Things to Consider

  • Law of Demeter - Passing into your function only what you need and not entire objects.
  • Breaking your function into segments, why not use methods to systematically get to your answer? Easily readable/maintainable.
  • Reduce the amount of variable creation, you dont need to set each value to a new variable when one is already passed in from the function arguments. Law of Demeter helps here, too.
  • Keeping your code expandable.

Here's an example of a function that follows the three above rules, a simple example that leaves you to put in your mathematics.

Example

var measure = function(elementHeight, elementPositionTop, bubbleHeight) {
    var remaining = 0, progress = 0, distance = 0;

    return {
        init: function() {
            measure.buildRemaining();
            measure.buildProgress();
            measure.buildDistance();
        },
        buildRemaining: function() {
            remaining = elementHeight * bubbleHeight;
        },
        buildProgress: function() {
            progress = remaining / 2.5;
        },
        buildDistance: function() {
            distance = (elementPositionTop - progress) + remaining;
        },
        result: function() {
            measure.init();
            return {
                remaining: remaining,
                progress: progress,
                distance: distance
            }
        }
    }
}($element.height(), $element.position().top, $bubble.height());

Accessing the Function

measure.result();

Returns

Object { remaining=5850, progress=2340, distance=3582 }
share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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