Using Google Closure Compiler to write better code

So I ran into a bit of code that I wrote a while back. I thought could be written a lot more efficiently. So I thought, hey Google's Closure Compiler is good at that stuff right? Let see what happens!

So, lets say we started out with this list of contacts:

var list = {
 'friends' : {
  'fred' : '123-4567',
  'lisa' : '123-6789'
 },
 'work' : {
  'joe'  : '234-5678'
 }
}

I need a function to add someone to the list, or update a number, but it need to check if the group already exists or if the person is already listed:

function addName(group, name, number){
 if (list[group] && list[group][name]){
  list[group][name] = number;
 } else {
  if (list[group]){
   list[group][name] = number;
  } else {
   list[group] = {};
   list[group][name] = number;
  }
 }
}

so the addName function starts out by seeing if the group exists, then if the name exists within that group:

if (list[group] && list[group][name]){

if that works, then just add/update the number. If not, then we drop through the else to the next set of evaluations. Does the group exist?

if (list[group]){

Yes? Add the number, if not, keep going to through the next else, from here we know that the group doesn't exist, so we define it, then add the name.

Seems efficient right?... hmm, but I see a lot of list[group][name] = number; in there. Lets see what Google's closure compiler does to it.

But first we need to keep the compiler from changing all the names. Lets pull out the code from within the function and force the compiler to keep our constants (see the first line):

/** @const */ var list = {}, group, name, number;

 if (list[group] && list[group][name]){
  list[group][name] = number;
 } else {
  if (list[group]){
   list[group][name] = number;
  } else {
   list[group] = {};
   list[group][name] = number;
  }
 }

The extra asterisk in the comment is important! (ref). Note, maybe there is a better way to force the compiler to keep your variable names within a function, but I don't know it.

Anyway, click the reset link and paste the code above into the compiler (use the "Simple" setting if it isn't already set). You'll end up with this:

var list={},group,name,number;if(!list[group]||!list[group][name])list[group]||(list[group]={});list[group][name]=number;

If it's too messy for you to clean up yourself, or you have a lot more code, copy that result and paste it into jsbeautifier and you'll end up with nicer formatted code.

Also, it's important to remember that compilers don't necessarily always use curly brackets after if or else statements, so the above code is equivalent to:

var list = {}, group, name, number;
if (!list[group] || !list[group][name]) {
  list[group] || (list[group] = {});
}
list[group][name] = number;

So now we can replace our function wrapper and remove the constants:

function addName(group, name, number){
  if (!list[group] || !list[group][name]) {
    list[group] || (list[group] = {});
  }
  list[group][name] = number;
}

That looks better, but I'm still seeing a bunch of list[group] in there. I'm sure the code can be a bit more DRY (don't repeat yourself). Also do we really need to check list[group][name]? It was checked initially but not checked again...

function addName(group, name, number){
  if (!list[group]) {
    list[group] || (list[group] = {});
  }
  list[group][name] = number;
}

Well now I see that list[group] is checked twice. Let's get rid of that and see what happens:

function addName(group, name, number){
  if (!list[group]) {
    list[group] = {};
  }
  list[group][name] = number;
}

Now lets see what we have. Check if list[group] exists, then add it if it doesn't. Add the number. Wow! Short and sweet. Why didn't I write it like that originally? Oh yeah, I'm still a noob :)

So after some testing, this new cleaned up function works exactly like the original! We went from 10 lines inside of the addName function down to 4! I'm happy, a happy noob!

I threw together this demo for testing.



I hope that my process of learning at least gives you some ideas. I don't claim to be an expert at javascript or jQuery, but I try to learn a little every day :)

As a redneck friend of mine once said, "Dem Google people is smart!"