Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 5 down vote favorite

I'm looking for a code review of some javascript I wrote - this code very simply counts in the direction of a particular target number, with the count slowing down as the target gets nearer.

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 - If anyone wants to point out an elegant way of doing this feel free, I'm planning on having a good think on it over the weekend. 

<!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>
share|improve this question
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? – Anonymous May 28 '11 at 0:02
@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. – Anonymous May 28 '11 at 0:04
If you're serious about learning better Javascript, here's a great place to start looking: javascript.crockford.com – Rice Flour Cookies Jun 29 '11 at 14:08

migrated from stackoverflow.com May 28 '11 at 0:31

1 Answer

up vote 2 down vote accepted 
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
share|improve this answer
1  
Hmm, I'd consider from < to ? from++ : from--; bad style. – RoToRa May 28 '11 at 18:31
@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. – Tomalak May 29 '11 at 7:41
How about from += (from < to) ? +1 : -1; ? – RoToRa May 30 '11 at 14:00
@RoToRa: If you think that's clearer. ;-) – Tomalak May 30 '11 at 14:04

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.