3
\$\begingroup\$

Would it serve much purpose to work at refactoring the latter parts of this code?

// This part is OK
function isScrolledIntoView(elem, trigger_height) {

  var docViewTop = $(window).scrollTop();
  var docViewBottom = docViewTop + $(window).height();
  var elemTop = elem.offset().top;

  if (trigger_height == false) {
    trigger_height = elem.height()
  }
  var elemBottom = elemTop + trigger_height;

  return ((elemBottom <= docViewBottom) && (elemTop >= docViewTop));
}

// This function happens when the page first loads
$("p").addClass('hidden').each(function() {
  $this = $(this);
  if (isScrolledIntoView($this, false)) {
    $this.removeClass("hidden");
  }
});

// This function happens on scroll, but basically repeats itself    
$(window).off('scroll').scroll(function() {
  $("p.hidden").each(function() {
    $this = $(this);
    if (isScrolledIntoView($this, false)) {
      $this.removeClass("hidden")
    }
  });
});

To me, it seems like there isn't – but at the same time its repetitive:

$this = $(this);
if (isScrolledIntoView($this, false)) {
  $this.removeClass("hidden")
}

Is there a general rule that anyone follows such as "only refactor if you're looking at > 5 lines of repeated code", or should you be refactoring absolutely everything as you go?

\$\endgroup\$
1
  • \$\begingroup\$ The real question is what you want. If you feel it should be re-factored, re-factor it. If you feel it's OK, leave it be. \$\endgroup\$
    – Mast
    Jun 16, 2015 at 10:37

1 Answer 1

2
\$\begingroup\$

I can't really see much to refactor in your code except a few things, so I'll go through those.


if (trigger_height == false)

could be if (!(trigger_height))


$this = $(this);
if (isScrolledIntoView($this, false)) {
    $this.removeClass("hidden");
}

I don't really see why you need to initialise $this, you could just use $(this) later on instead.

\$\endgroup\$

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.