6
\$\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
    Commented May 28, 2011 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
    Commented May 28, 2011 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
    Commented Jun 29, 2011 at 14:08

1 Answer 1

Reset to default
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
    Commented May 28, 2011 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
    Commented May 29, 2011 at 7:41
  • \$\begingroup\$ How about from += (from < to) ? +1 : -1; ? \$\endgroup\$
    – RoToRa
    Commented May 30, 2011 at 14:00
  • \$\begingroup\$ @RoToRa: If you think that's clearer. ;-) \$\endgroup\$
    – Tomalak
    Commented May 30, 2011 at 14:04

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.