0
\$\begingroup\$

I wrote this small test. I would love to know if it's possible to achieve the same result in a more "efficient" way. Before any moderator closes this question, it's not subjective because my definition of efficient here means less lines of code and less memory usage.

Here is a demo: http://jsfiddle.net/YsjLp/

And here is the JavaScript code:

var position = 0;
var divToEdit = document.getElementById('here');

function executeIt(){
    setInterval(function(){moveIt()},300);  
}

function moveIt(){
    var textToInsert = '';
    var textLength = 10;

    for (var i = 0; i <= textLength; i++) {     
        
        if(i == position){                      
            textToInsert += '[-]';      
        }else{
            textToInsert += '-';    
        }       
    };  

    if(position == textLength){
        position = 0;
    }else{
        position += 1;
    }
    
    divToEdit.innerHTML = textToInsert;
}   

executeIt();
\$\endgroup\$
2
  • \$\begingroup\$ Is there even a question here? \$\endgroup\$
    – user25057
    Sep 4, 2013 at 5:36
  • \$\begingroup\$ @remyabel yes, the question is, could the above code be rewritten in a more efficient way, efficient defined in the question body. \$\endgroup\$
    – Taleb Elem
    Sep 4, 2013 at 5:38

2 Answers 2

1
\$\begingroup\$

As you define efficient to be "less lines of code", here it is:

function moveIt(){
    var textToInsert = '';
    var textLength = 10;

    for (var i = 0; i <= textLength; i++)
        textToInsert+= (i==position)?'[-]':'-';
    (position==textLength)?position=0:position++;
    divToEdit.innerHTML = textToInsert;
}

Demo: http://jsfiddle.net/YsjLp/3/

\$\endgroup\$
1
  • \$\begingroup\$ Fair enough, a good use of ternary operators, i would love to see both benchmarked, i'll check and see, thank you. \$\endgroup\$
    – Taleb Elem
    Sep 4, 2013 at 21:07
2
\$\begingroup\$

Some tips:

  • Use const and let over var, they are simply better.
  • Readability pretty much always trumps 'efficiency'
  • Use textContent if you are not actually inserting HTML
  • You can use parameters to make your code more flexible

const targetDiv = document.getElementById('target');
const chars = '[-]';
const bgChars = '-';
const length = 20;
const intervalMs = 200;

let position = 0;
const moveCharsInText = (
  targetElement,
  charsToMove,
  backgroundChars,
  sectionCount
) => {
  let textToInsert = '';

  for (let i = 0; i < sectionCount; i++) {
    if (i === position) {
      textToInsert += charsToMove;
    } else {
      textToInsert += backgroundChars;
    }
  }

  position++;
  if (position >= sectionCount) {
    position = 0;
  }

  targetElement.textContent = textToInsert;
};

const start = () => {
  setInterval(() => {
    moveCharsInText(targetDiv, chars, bgChars, length);
  }, intervalMs);
};

start();
<div id="target"></div>

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.