4
\$\begingroup\$

Review this code for code quality.

<!DOCTYPE html>
<title>Email obfuscator</title>
<style>.email { unicode-bidi: bidi-override; direction: rtl; }</style>
<h1>Email obfuscator</h1>
<p>A kick-ass email obfuscator, inspired by <a href="http://mathiasbynens.be/demo/email-obfuscator#code">Mathias Bynens's <code>obfuscate_email()</code> PHP function</a>. This tool will encode (HTML) and/or reverse (CSS) any email address you enter, making it less vulnerable to spammers who use email harvesting software aka spambots.</p>
<h2>The output</h2>
<p id="preview">(preview will be shown here)</p>
<p><textarea id="output" rows="6" cols="60" onclick="this.select()"></textarea></p>
<h2>The input</h2>
<form name="OBFUSCATOR"> 
  <p><label for="email">Email: <small>(enter the email address to be obfuscated)</small></label> <input type="text" id="email" size="28" autofocus></p>
  <p><input id="encode" type="checkbox" checked> <label for="encode">Convert into HTML entities ("encode")</label></p>
  <p><input id="reverse" type="checkbox"> <label for="reverse">Reverse using CSS</label></p>
  <p><input id="link" type="checkbox"> <label for="link">Create HTML link</label></p>
  <p><input type="button" value="Obfuscate This!" onclick="obfuscate_email()"></p>
</form>
<script>
function obfuscate_email() {
  var email = document.OBFUSCATOR.email.value,
    encodedEmail = "",
    output = "";

  if (!email) {
    alert("Please enter an email address.");
  } else {
    if (document.OBFUSCATOR.reverse.checked) {
      email = email.split("").reverse().join("");
      output = "<span class=\"email\">";
    }
    if (document.OBFUSCATOR.encode.checked) {
      for (var i = 0; i < email.length; i++) {
        encodedEmail += "&#" + email.charCodeAt(i) + ";";
      }
      output += encodedEmail;
    } else {
      output += email;
    }
    if (document.OBFUSCATOR.reverse.checked) {
      output += "</span>";
    }

    document.getElementById("output").value = document.OBFUSCATOR.link.checked ? "<a href=\"&#109;&#97;&#105;&#108;&#116;&#111;&#58;" + (document.OBFUSCATOR.encode.checked ? encodedEmail : email) + "\">" + output + "</a>" : output;
    document.getElementById("preview").innerHTML = document.getElementById("output").value;
  }
}
</script>
\$\endgroup\$
0

1 Answer 1

4
\$\begingroup\$

Your code has room for improvement,

in top-bottom order:

  • Your code is littered with DOM queries, I would suggest to group them all on top, they reduce comprehension speed
  • Access elements through the form is considered old skool, always use getElementByID
  • Adding a span around the email should be done in 1 go, it would be much cleaner
  • You can determine the output in one shot, from the email address
  • If continuing does not makes sense ( when email is blank ), return early, this will reduce arrow coding

If you apply all this you should end up with something like this:

function obfuscateEmail( )
{
  var email = document.getElementById( 'email' ).value,
      encode = document.getElementById( 'encode' ).checked,
      reverse = document.getElementById( 'reverse' ).checked,
      link = document.getElementById( 'link' ).checked,
      output = document.getElementById( 'output' ),
      preview = document.getElementById('preview'),
      html = '',
      encodeEmail = '';

  if( !email ) {
    return alert('Please enter an email address.');
  }

  if( reverse ) {
    email = email.split('').reverse().join('');
  }
  if( encode ){
    for (var i = 0; i < email.length; i++) {
      encodeEmail += '&#' + email.charCodeAt(i) + ';';
    }       
    email = encodeEmail;
  }

  html = reverse?( '<span class="email">' + email + '</span>' ):email;

  output.value = link ? "<a href=\"&#109;&#97;&#105;&#108;&#116;&#111;&#58;" + email + "\">" + html + "</a>" : html;
  preview.innerHTML = output.value;  
}
\$\endgroup\$
1
  • \$\begingroup\$ thanks for the answer. but I think you forget to replace output with html here - link ? "<a href=\"&#109;&#97;&#105;&#108;&#116;&#111;&#58;" + email + "\">" + output + "</a>" : output \$\endgroup\$
    – user33979
    Dec 24, 2013 at 17:56

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.