Everyone has their own style of programming and I am not attempting to change that here. That being said, some practices lead to unreadable and unmaintainable code. Below is a list of coding practices that I often times see being employed and are generally a bad idea. For each practice I will describe what is being done, why it is wrong, and give an example of the correct way of doing things. Bear in mind that I am guilty of some of these myself so I think we all have room to grow in this regard.
Nested Ternary Operators
The ternary operator can be a quick way to conditionally use different values in an expression. The basic cases for this are useful and easy to understand but there is a significant potential for abuse. The easiest abuse case to point out is when a ternary operation is nested within another ternary operation. When this happens it is very difficult to understand what the original programmer intended (even if you are the original programmer) which makes troubleshooting complicated.
The wrong way:
var result = conditional1 ? conditional2 ? value1 : value2 : value3;
I have seen much worse than this but even this is difficult to understand. A better way, still using the ternary operator:
var interimResult = conditional2 ? value1 : value2;
var result = conditional1 ? interimResult : value3;
There are other, more efficient, ways of doing this but you get the point.
Nested Anonymous Functions
Anonymous functions are a shortcut in javascript that can be useful if the function is used only in one place (such as with a callback). Without careful attention to formatting it can be difficult to tell where one function ends and the other begins. This problem is multiplied exponentially when Anonymous functions are nested or you are passing multiple functions in as parameters to a function.
The wrong way:
var result = getResult(
function(param1, param2){
var innerResult = getInnerResult(
function(innerParam1){
...do stuff
...return something
}
)
return innerResult;
}
);
This is an over-simplified example but you can see how things can become confusing. This example is formatted far better than is generally the case and it would still be confusing as to where the responsibility of each function begins and ends. I understand that nesting functions is the only way to encapsulate a private scope (closure) in Javascript but I find that often times nested functions are used as a convenience instead of in the context of a closure.
A better way:
function function1(innerParam1){
...do stuff
...return something
}
function function2(param1, param2){
var innerResult = getInnerResult(function1);
return innerResult;
}
var result = getResult(function2);
Both are the same amount of code, just separated differently and the second way is far easier to understand and maintain.
Overly Abbreviated Naming
Abbreviating variable names is an old tradition that dates back to the early days in programming when every character mattered. Nowadays it doesn’t so when you abbreviate a variable or function name you are trading code readability for the ability to type less characters when you are using the variable. While this sounds like a good idea up front, it often comes back to bite you later on. Additionally, most IDEs now will find the variable you are looking for after you type the first few characters anyway so speed is no longer a good excuse.
The wrong way:
//Uh, what does dyh stand for again?
var dyh = result;
If you were lucky enough to be the original coder of the line above then you have a chance of figuring out what it means. Otherwise you don’t without either guessing or asking the original programmer. This problem is worse in JavaScript because you don’t have strict typing to help you out.
The right way:
var driverYearHeat = result;
Now it is obvious what we are talking about. Bear in mind, I am not saying that all abbreviation is bad. There are certain common abbreviations that are acceptable. Also, abbreviations where it is still obvious what you are talking about is fine as well. Below is an incomplete list of common abbreviations:
- i – Commonly used as the index of a for loop iteration
- obj – Object
- num – Number (as in number of, ie: numDrivers)
- tpl – Template
- impl – Implementation
- j – The inner index of a for loop iteration
If you are unsure, just spell it out. It’s not that big of a deal and will probably save you time later.
Using JQuery instead of Angular Directives
JQuery is great but should not be invoked directly on a DOM element when in angular. There are a few reasons for this but the main ones are ease of unit testing and reusablity. Instead, directives should be used.
Now, this is not to say that JQuery functions should not be used. Just you should not target an element of the DOM directly by ID, class, or tag. The $element parameter of a directive contains the DOM element that the directive encapsulates. This is a JQuery element and as such all JQuery functions can be invoked from it.
The wrong way:
<div id="myDiv" />
<script>
$('#myDiv').hide();
</script>
The right way:
<div id="myDiv" hide="true" />
<script>
angular.module().directive('hide', [ function () {
return {
restrict: 'A',
controller:['$scope', '$element', '$attrs',
function ($scope, $element, $attrs) {
if($attrs.hide){
$element.hide();
}
}]
}
}]);
</script>
Again, an oversimplified example. Note that the JQuery example is a lot less code, but you would need to invoke that specifically anywhere you needed this functionality. With the Angular version, just use the directive and you get the rest of the functionality without additional lines of code.
Conclusion
I’m sure as we mature as a company and AngularJS developers our coding standards will mature as well. As such, I expect that this list will grow and change.