patternjavascriptMinor
l33t Speak translator
Viewed 0 times
translatorl33tspeak
Problem
Please review the code quality of this l33t Speak translator. Improvement suggestions are welcome. Here it is - http://jsbin.com/IHoFuQE/1
(function() {
"use strict";
// http://en.wikipedia.org/wiki/Leet
// http://www.catb.org/jargon/html/crackers.html
var alphabets = { // common
"a": "4",
"b": "8",
"e": "3",
"f": "ph",
"g": "6", // or 9
"i": "1", // or |
"o": "0",
"s": "5",
"t": "7" // or +
},
alphabets2 = { // less common
"c": "(", // or k or | 0.5) {
text = text.replace(text[i], text[i].toUpperCase());
} // else keep lower case
}
return text;
}
(function() { // l33t the words object
for (var word in words) {
if (words.hasOwnProperty(word)) {
words[changeLetters(word)] = words[word];
delete words[word];
}
}
}());
function tol33t() {
leet.value = randomcase.checked ? randomizeCase() : changeWords();
}
elite.addEventListener("input", tol33t);
btn.addEventListener("click", tol33t);
}());Solution
To start with, I'd like to give you credit for using
Your
There should be clearer separation responsibility between the DOM-interaction and text-translation layers of code. The text translation code should reside in pure functions (whose results depend solely on their parameters) that you could call from the JavaScript console or from a unit testing engine. That separation of concerns could be accomplished with:
Your "l33t the words object" initialization routine lays a trap for anyone trying to read the code. When code in
I would write
… which is actually analogous to your implementation of
"use strict" and limiting the scope of variables using functions.Your
tol33t() function is weird to me. If the randomcase button is checked, you call randomizeCase() with no arguments. What are you randomizing the case of? It turns out that randomizeCase() calls changeWords(), so it turns out that the two code branches have something in common after all. That wasn't apparent from looking at tol33t().There should be clearer separation responsibility between the DOM-interaction and text-translation layers of code. The text translation code should reside in pure functions (whose results depend solely on their parameters) that you could call from the JavaScript console or from a unit testing engine. That separation of concerns could be accomplished with:
function onTranslateButtonClicked() {
var l33tText = toL33t(elite.value, adv.checked);
if (randomcase.checked) l33tText = randomizeCase(l33tText);
leet.value = l33tText;
}Your "l33t the words object" initialization routine lays a trap for anyone trying to read the code. When code in
changeWords() refers to the words associative array, one would reasonably think that it's using the definition above (i.e., words = { "am": "m", … }) instead of the munged version (words = { "4m": "m" }). Better to make that munging explicit when you define words (and do so by returning a copy rather than mutating the dictionary as you iterate):var words = l33tKeys({
"am": "m",
…
});
function l33tKeys(dict) {
var l33tDict = {};
for (var word in dict) {
l33tDict[changeWord(word)] = dict[word];
}
return l33tDict;
}I would write
changeLetters() as:// letterSubsts may be alphabet or alphabet2
function changeLetters(text, letterSubsts) {
return text.toLowerCase().replace(/[a-z]/g, function(letter) {
return letterSubsts[letter] || letter;
});
}… which is actually analogous to your implementation of
changeWords().Code Snippets
function onTranslateButtonClicked() {
var l33tText = toL33t(elite.value, adv.checked);
if (randomcase.checked) l33tText = randomizeCase(l33tText);
leet.value = l33tText;
}var words = l33tKeys({
"am": "m",
…
});
function l33tKeys(dict) {
var l33tDict = {};
for (var word in dict) {
l33tDict[changeWord(word)] = dict[word];
}
return l33tDict;
}// letterSubsts may be alphabet or alphabet2
function changeLetters(text, letterSubsts) {
return text.toLowerCase().replace(/[a-z]/g, function(letter) {
return letterSubsts[letter] || letter;
});
}Context
StackExchange Code Review Q#39365, answer score: 5
Revisions (0)
No revisions yet.