Variables Naming Saves Lives

Let’s take a closer look at variable naming

I recently got put onto a project with code from another dev house.

The problem I have with the code isn’t the fact that it’s from another dev house. It’s usually got to do with the fact that the processes, testing, variable naming etc… are completely different, or non-existent.

In my case it was quite clear that they had no processes or standards in place. Obviously there were no tests either. Needless to say there have been points where I was completely overwhelmed.

Fortunately I happened upon this talk by Martin Fowler (which I really enjoyed and think is definitely worth watching), where he goes into his ideas on refactoring. He reminded me that small incremental changes go a long way in improving code.

I would now like to take you through the process I went through of refactoring code by simply changing variable names. This will hopefully show you how changing variable names can be a game changer (and save lives by reducing cardiac arrest cases :P)

Okay, so below is the blob of code that was left behind.

Cardiac arrest waiting to happen

WTF.

Yeah that was my thought too. Where do I even begin? At the beginning of course — Line 1:

Object.keys(data).sort().map((item, key) => {

So we’re taking an object called data (WTF IS DATA?), sorting it and mapping over each of its keys (which has been called item — WTF IS ITEM?).

I hate the word data in this case because it is not apparent what this data is. This becomes doubly worse when you use item because now you don’t know anything about two variables.

Hence I did what I think everyone should do and renamed data and item into more relevant names:

Object.keys(productionDays)      
  .sort()      
  .map((productionDay) => {

It turned out that data was in fact all the days of a production, hence productionDays. It therefore makes sense to call an item of productionDays a productionDay. I removed the key because it wasn’t actually necessary. (I find that moving the different functions into different lines also really improves the readability of code as is the case in the above — usually a linter will do this for you automatically, which is something I highly recommend).

Now that we actually know what the context is that we are working in we can march forward!

Line 2:

const grouped = {};

Grouped by what? What is this a grouping of? Who falls into this group? So many questions so little time.

Refactor:

const groupCastMembersByRoleType = {}

Makes more sense right? Especially considering we are dealing with a production.

Line 3:

data[item].map(item => {

Here we are looking at the:

1. The data object,
2. Finding an object in data that has a key of item
3. Mapping over that object using the name item

So you have an item of an item. Makes TOTAL SENSE!?

Refactor:

productionDays[productionDay].map((castMember) => {

Here we are looking at the:

1. productionDays object
2. We are finding a productionDay in productionDays
3. Mapping over a productionDay using the name castMember

You now have a castMember of a productionDay. Makes a bit more sense right?

Line 4:

if (typeof grouped[item.type] === 'undefined') {

We now have a conditional where we assess whether grouped[item.type] is undefined. I don’t know about you but I have no idea what this conditional is trying to check just by looking at it. This for me is an issue.

Refactor:

if (            
  typeof groupCastMembersByRoleType[castMember.type] === 'undefined'          ) {

I don’t think this is a brilliant piece of work, but I do feel that you have a bit more context of what this is actually trying to check. If the grouping of cast members by role type contains a castMember type that doesn’t exist then do …

Moving along to what the conditional should execute on true.

Line 5–9:

{ 
  grouped[item.type] = { 
    name: item.type,            
    members: [item]          
  };        
}

Grouped suddenly has names and members —LOGICAL!?

Refactor:

{            
  groupCastMembersByRoleType[castMember.type] = {                   
    roleType: castMember.type,              
    castMembers: [castMember],            
  }
}

If conditional evaluates to true, we now assign to the grouping of cast members by role type a role type (which is the castMember type) and cast members which is an array that takes in a cast member. Like I said earlier not the best but a lot more descriptive in my opinion.

For the next one I’m gonna just let you compare the two:

}        
  else {          
    grouped[item.type].members.push(item);        
  }      
});

Refactor:

} 
  else {                
    groupCastMembersByRoleType[castMember.type].castMembers.push(                                 
      castMember,            
    )          
  }        
})

Finally, let’s compare the returns:

return ({        
  key: key + 1,        
  name: item,        
  roles: Object.keys(grouped).map(key => ({          
    name: grouped[key].name,          
    members: grouped[key].members.map(member => ({            
      role: member.role,            
      company: member.company,            
      name: member.name,            
      number: member.number,            
      email: member.email,            
      description: member.description,          
    }))        
  }))      
});

We’re returning

1. key (which we remove because it isn’t actually used anywhere — go figure)
2. name — but we’re not really sure what this is because item!?
3. roles — finally we get an inkling of what this is actually for

Refactor:

return {          
  name: productionDay,          
  roleTypes: Object.keys(groupCastMembersByRoleType).map(              
    (roleType) => ({              
      roleType: groupCastMembersByRoleType[roleType].roleType,                    
      castMembers:         
        groupCastMembersByRoleType[roleType].castMembers.map(                   
          (member) => ({                  
            role: member.role,                  
            company: member.company,                  
            name: member.name,                  
            number: member.number,                  
            email: member.email,                  
            description: member.description,                
         }),              
       ),            
    }),          
  ),        
}

We eventually figured out that we want to return an object that has a name and roleTypes. Where name is the name of the the productionDay and the roleTypes is an array of cast members grouped by their role type.

Hence the point of this function is to take all the cast members in a production, group them into a production day which will itself contain the cast members grouped by their specific role type. This is the power of variable names!

AND HERE IS THE FINAL PRODUCT!

I survive to fight another day (my heart is grateful :P).

I hope you learned something from this exercise, here’s my list of learnings:

1. Don’t use item or data unless it’s explicit what these things represent

2. Intuitive variable naming can save loads of time for new developers entering a project or for that time when you yourself will need to review the code for whatever reason

3. Using correct and precise variable naming is a standard that should not be dropped for any reason. ( You may not pay for it now, but you or your colleague will in the future — and karmas a bitch!)

4. Naming variables literally saves lives — if you have to look at the initial code I put forth in the above and you’re in a time precious situation you’re screwed -> cardiac arrest

Now I can drink my coffee and increase my heart rate without fear! :D