Compiler for grond, a language-in-the-making I'm working on a language called grond (see it on GitHub); the compiler is so small it is included in this post.

My gut tells me that this code is awful, but it works. I am hesitant to change things to make the code better because generally, I have found that bad things happen when I "fix" something that isn't necessarily broken.

How can I make this "ugly" code better? Note - If you don't think this code is ugly, please tell me!



String.prototype.reverse = function() {
  return this.split("").reverse().join("");
}

function replace(wc) {
  var lastWC;

  while (true) {
    lastWC = wc;
    wc = wc.reverse();
    wc = wc.replace("§", "");
    wc = wc.replace("Ś", "");
    wc = wc.replace(/\(a(?!\w)/, "(trela"); //alert ,a
    wc = wc.replace(/\(f(?!\w)/, "(rof"); // for, f
    wc = wc.replace(/\(w(?!\w)/, "(elihw"); //while,w
    wc = wc.replace(/\(p(?!\w)/, "(tpmorp"); //prompt,p
    wc = wc.replace(/\(vr\./, "(esrever."); //reverse, rv
    wc = wc.replace(/\(tm\./, "(chtam."); // match, mt
    wc = wc.replace(/\(nj\./, "(nioj."); //join, .jn
    wc = wc.replace(/\(pr\./, "(ecalper."); //replace, .rp
    wc = wc.replace(/\(ls\./, "(.ecils"); //slice, sl
    wc = wc.replace(/\(lt\./, "(esaCrewoLot."); //toLowerCase, tl
    wc = wc.replace(/\(ut\./, "(esaCreppUot."); //toUpperCase, tu
    wc = wc.replace(/\(ps\./, "(tilps."); //split, sp
    wc = wc.reverse();
    wc = wc.replace(/\.l(?!\w)/, ".length"); // .length, .l
    if (lastWC === wc)
      break;
  }
  return wc;
}

var source = prompt("Enter Source");
var sourceCopy = source;


var fullASCII = ' !#$%&amp;()*+,-./0123456789:;&lt;=&gt;?@ABCDEFGHIJKLMNOPQRSTUVWXYZ[\\]^_`abcdefghijklmnopqrstuvwxyz{|}~'.split('');
var y = (a) =&gt; {
  for (let char in fullASCII) {
    if (!source.contains(char)) {
      source.replace(a, char);
    }
  }
}
var OK = false;
var OK2 = false;
for (let char in fullASCII) {
  if (!source.contains(char)) {
    OK2 = OK ? true : false;
    OK = true;
  }
}
if (!OK2) {
  throw "Your program uses every ASCII charactor. Grond requires your program to leave 2 ASCII chars unused. This is stupid and will be fixed.";
}
y('\\\"');
y("\\\'");
var sourceSplit = sourceCopy.split("\'");
for (let i = 0; i &lt; sourceSplit.length; i += 2) {
  var o = sourceSplit[i].split('\"');
  for (let e = 0; e &lt; o.length; e += 2) {
    o[e] = replace(o[e]);
  }
  sourceSplit[i] = o.join('\"');
}
sourceCopy = sourceSplit.join("\'");

var append = "if(s != \"\"){alert(s)}";
var prepend = "var s = prompt();";
var oSS = source.split('');
if (oSS[0] == "§") {
  prepend = "var s = \"\"";
}
if (oSS[0] == "Ś" || oSS[1] == "Ś") {
  prepend = "with(math){" + prepend;
  append += "}";
}

sourceCopy = prepend + " String.prototype.la=function(h){var x=this.split(''); x[h] = x[h].toLowerCase(); return x.join('');};String.prototype.ca=function(h){var x=this.split(''); x[h] = x[h].toUpperCase(); return x.join('');};String.prototype.reverse=function(){return this.split('').reverse().join('');}; \n var h = ['hello, world', 'Hello, World', 'Hello, World!', 'hello, world!', 'hello world', 'hello world!', 'Hello World', 'Hello World!', 'Hello, world.', 'Hello, World.' ,'hello world.' , 'Hello World.'];  var q ='" + source + "'; " + sourceCopy + append;
alert(sourceCopy);
eval(sourceCopy);
String.prototype.reverse = function() {
  return this.split("").reverse().join("");
}



Firstly, don't modify objects you don't own. It could cause incompatibility problems if some other code had its own String.prototype.reverse function which had different functionality.

Secondly, this is a bad implementation of reversing a string. For example, calling this function on the following string:

'foo  bar'


returns:

'rab �� oof'




You use many meaningless identifiers, for example wc, y, OK, OK2. It's hard to tell what purpose they serve.




while (true)



Instead of using an infinite loop and break-ing it, I recommend you using a do-while loop. In your case it could look like that:

var condition = false
do {
  // the actual loop content

  condition = lastWC === wc
} while (!condition)


See Is while (true) bad programming practice?.



I highly recommend you wrapping your code in an IIFE (see Ben Alman's article for more details). In the current state, all your variables are global, which is a bad practice. See Why are global variables considered bad practice?.



I had to spend quite a while to work out how the replace function works. It seems that you wanted to re-invent the g regex modifier (read MDN RegExp docs). You could get rid of the loop at all if you used it.

I don't see any point in reversing the wc variable two times, except of making the code less readable.

Calling wc.replace() multiple times and assigning the result to wc looks ugly. You could at least chain the .replace() calls like that: wc.replace(...).replace(...).replace(...), or even better, make an array of matches and replacements and loop through them:

var matchesAndReplacements = [
  ["§", ""]
 ,["Ś", ""]
 ,[/\(a(?!\w)/g, "(trela"]
]

for (let i of matchesAndReplacements) {
  let [match, replacement] = i
  wc = wc.replace(match, replacement)
}


And most importantly, this doesn't work. Writing a programming language compiler using regex is a very naive idea, and IMHO even worse than trying to parse HTML with regex. A simple example: what if you had a .l fragment in a string? It would be incorrectly changed to .length.




var source = prompt("Enter Source");



prompt() is a pretty primitive way to make an I/O. Even if you decide so, it shouldn't be somewhere in the middle of the code. Create a function, named compile for example, which takes input as argument and returns output, and then at the end of the code place alert(compile(prompt("Enter Source"))).




var source = prompt("Enter Source");
var sourceCopy = source;



When you declare multiple variables at once, you can use the var keyword only once, and separate the variables declarations with a comma, like that:

var source = prompt("Enter Source")
   ,sourceCopy = source


However, in this case it would be better to use const for source, as you don't intend to modify it.




var y = (a) =&gt; {
  for (let char in fullASCII) {
    if (!source.contains(char)) {
      source.replace(a, char);
    }
  }
}



If your arrow function has only one parameter, you don't need to use parentheses, so it could be var y = a=&gt; {}.

Also, this function seems to be no-op, because it doesn't return any value and doesn't assign any value to any variable.



var OK = false;
var OK2 = false;
for(let char in fullASCII){
  if(!source.contains(char)){
    OK2 = OK? true:false;
    OK = true;
  }
}


That looks ugly for me. You could rewrite it like that:

const isOK = fullASCII.map(source.contains.bind(char)).filter(x=&gt;x).length &gt;= 2


One liners FTW!




var sourceSplit = sourceCopy.split("\'");



You don't need to escape apostrophes if you use double quotes to make a string. Besides, I would name the variable sourceSplitted, because now it looks more like a function name.




if (oSS[0] == "§") {



Always use === instead of ==. See Does it matter which equals operator (== vs ===) I use in JavaScript comparisons?.



To conclude: yes, this code is ugly. You have a lot to learn.