This came up recently on my team. When making changes to pre-existing code, it is tempting to bring the code in line with our personal tastes. If we like Allman style braces, we’ll change the code away from K&R bracing. If we don’t like Hungarian, we will be tempted to change the variable names by removing the annotations. There are many more styles I could mention here. When faced with the temptation to change this code, resist. Unless you are working by a coding standard that clearly calls out your style, don’t change things. Even if your coding conventions do mention a style, consider ignoring the infraction. There are two reasons for this.
First, every change in the code is a chance to break something. The code you are working with is known to be working. Well, mostly. There is some reason you are in there and it might be to fix a bug. Even so, most of the code is known to be working. If you change it–even for something small like bracing styles–you risk breaking something. Making a change that subtly changes the code flow. Unit tests will help reduce the risk here but unit tests don’t cover everything. Remember, we write code to deliver features to end users. Why risk introducing a bug unless you are adding something of value to that end user?
Second, it is just a bad policy to change the code. It makes code reviewing harder because it’s hard to see the real changes from the purely cosmetic ones. Even if you check in the changes in a different checkin, someone trying to following the history of the code will still come across your changes. Worse, what happens when the next person to touch the code prefers K&R bracing and changes it back? In theory, the code could flip-flop every other checkin.
The moral of the story: Don’t make cosmetic code changes. Change code only because a) it is broken or b) changing it allows you better implement a new feature.