5
\$\begingroup\$

I'm looking for (and be as brutal as you like) ways to improve the code or the algorithm (I'm aware there should be comments) - I'm a recreational programmer and would like to be improving my skills. My next step is to get the targetNumber value from a nearby file without losing the current count.

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN "http://www.w3.org/TR/html4/strict.dtd"> 
<html> 
<head> 
  <title>Count to a particular target</title> 
</head> 
<body> 
<h1 id="myDiv">Starting</h1> 
<script type="text/javascript">
    currentValue = 100;
    targetValue = 1500;

    function count() {
        if (currentValue > targetValue) {
        currentValue -= 1
        } else if (currentValue < targetValue) {
            currentValue += 1
        } 
        document.getElementById('myDiv').innerHTML = 'Total wordcount:'+ currentValue.toString();
        changeTime = 20;
        if (Math.abs(currentValue - targetValue) < 980) {
            changeTime = 1000 - Math.abs(currentValue - targetValue);
        }
        setTimeout(count,changeTime/2);
    }
count()
</script> 
</body> 
</html> 
\$\endgroup\$
3
  • \$\begingroup\$ Whoops - did not know there was such a thing blush - what's the protocal here, do I delete and repost or will it just get transfered by a passing admin? \$\endgroup\$ – Anonymous May 28 '11 at 0:02
  • \$\begingroup\$ @Joe Reddington I would just delete and repost over there. We don't have the ability to vote for this to be migrated to codereview at the moment. \$\endgroup\$ – Anonymous May 28 '11 at 0:04
  • \$\begingroup\$ If you're serious about learning better Javascript, here's a great place to start looking: javascript.crockford.com \$\endgroup\$ – Vivian River Jun 29 '11 at 14:08
4
\$\begingroup\$
function count(from, to, targetElem) {
    targetElem.innerHTML = 'Total wordcount: ' + from;

    if (from == to) return;

    from < to ? from++ : from--; 

    var changeTime = Math.max(20, 1000 - Math.abs(from - to)) / 2;

    setTimeout(function() {count(from, to, target);}, changeTime);
}

count(50, 0, document.getElementById("t"));

A few notes:

  • avoid global variables to conserve state - use function arguments instead
  • always use var to declare variables. Otherwise they are in the global scope and you don't want this
  • added error checking (i.e. whether values are numeric and target is valid) might not be a bad idea, I left it out for clarity here
  • return early on known conditions, this reduces complexity in the function body
  • the ternary operator condition ? ifTrue : ifFalse can be used in other ways than assigning a value
\$\endgroup\$
4
  • 1
    \$\begingroup\$ Hmm, I'd consider from < to ? from++ : from--; bad style. \$\endgroup\$ – RoToRa May 28 '11 at 18:31
  • \$\begingroup\$ @RoToTa: Yes, you could say that. It's not really clean. Then again, it's crystal-clear what it does. An if (from < to) from++ else from--; would also be posible - I just wanted to stay on one line. \$\endgroup\$ – Tomalak May 29 '11 at 7:41
  • \$\begingroup\$ How about from += (from < to) ? +1 : -1; ? \$\endgroup\$ – RoToRa May 30 '11 at 14:00
  • \$\begingroup\$ @RoToRa: If you think that's clearer. ;-) \$\endgroup\$ – Tomalak May 30 '11 at 14:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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