Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

While developing in a group atmosphere and adding or modifying features in some code base. Is it considered offensive or impolite to reformat the previous developers code to bring it up to current coding standards? I understand that standards have changed, and will probably continue to change, but would any of you get offended if someone came through and changed your code formatting?

To be clear, I'm not talking about changing any logic, just messing with tabs and spaces and such.

EDIT: I don't only do this for the sake of coding standards, it helps me read through their code and bring it up to date so I can fully understand the logic that has been implemented before I start modifying critical applications.

share|improve this question
6  
Enable automatic "format at save" for everybody. Everbody use the same agreed upon settings. After a while all code is normalized. –  user1249 Jul 8 '11 at 18:03
    
There can be a point where this goes to far. I had a coworker that reformatted everything added linebreaks not that were needed or even relevant as far as I was concerned. Personally unless it is unreadable or the code has become my primary responsibility I leave the formatting alone unless I am making other changes. –  Chad Jul 8 '11 at 18:52
4  
Is this "I'm changing formatting... because I think it should look different".. or is this "I'm changing the formatting... too match the standards"... hugely different questions –  WernerCD Jul 8 '11 at 22:22
1  
@Thorbjorn I wouldn't consider a branch that fixes formatting in every file, 1 file per commit, killing history. However, fixing it during the same commit is just bad. (I guess they could use something like git add to selectively commit parts, but my guess is that most people use the equivalent of svn commit or git commit -a) –  alternative Jul 8 '11 at 22:43
1  
@mathepic, in that case then do a global reformat. Depends on your preferences. –  user1249 Jul 9 '11 at 6:23
show 4 more comments

10 Answers 10

up vote 17 down vote accepted

I think this is OK as long as the standards are agreed upon. One note of caution however; be aware if there is the potential the file is being modified by others at the same time. If you make their merge harder just because you were changing formatting you won't be very popular.

share|improve this answer
9  
The first sentence here is important. Make sure that you are actually following the agreed-upon standards, and not just making changes because you like them. –  Thomas Owens Jul 8 '11 at 17:00
add comment

Yes, the code should belong to the project. Bringing the code up to standard will help reduce the project's technical deficit. If you are modifying it, you are currently responsible for it. For older code, the original developer may no longer be on the project or has new duties.

When you do this kind of change it is good to run the verification tests after your reformatting. If they pass, check the code in before doing your feature changes.

EDIT: In the context of this question, reformatting to standard is appropriate. In the absence of standards, I would recommend advocating for standards and not reformatting until there are standards for the format. Reformatting to personal taste/standards should not be done with code belonging to the project.

share|improve this answer
2  
+1 for "check the code in before doing your feature changes" –  Blaise Doughan Jul 8 '11 at 17:01
1  
+1 again for "check in formatting changes before doing your feature changes" and "it is good to run the verification tests after your reformatting". Ideally, we should run verification tests before each checkin. –  leed25d Jul 8 '11 at 17:45
    
Actually, whether you reformat before or after the changes does not really matter. What matters is that aesthetical patches should be kept separate from functional patches --> if an aesthetical patch changed the functionality, it was not intended and can be deemed a bug; it makes functional patches easier to review (because smaller). –  Matthieu M. Jul 8 '11 at 18:24
    
@Matthiew M: True, but in most cases they will be done first to improve maintainability before maintenance. Few developers have time to do so after the fact. Also, if the code needs to be upgraded to pass automated check-in tests, it must be reformatted first to maintain separation of aesthetic and functional patches. –  BillThor Jul 10 '11 at 6:07
add comment

I believe it's always a good practice to refactor the code when you are modifying/adding to a particular file. That includes updating the style of the code to reflect proper naming conventions for variables/methods and coding styles.

share|improve this answer
    
The OP asked about reformatting, not refactoring. –  quant_dev Jul 8 '11 at 17:13
    
I know; I did say I consider that to also include reformatting :) –  Wayne M Jul 8 '11 at 18:06
add comment

I do this all the time. Old code should be held to the same standards of new code and if you don't fix it up while you are working on it no one will. I think this counts under the Boy Scout Rule.

share|improve this answer
add comment

I think this is good practice and a necessary part of code maintenance.

I would recommend checking in the formatting changes in one commit to the version control system and the functional changes in a separate commit to help both yourself and others understand what took place.

share|improve this answer
1  
+1 for separate commits. Trying to figure out what code changes were made in a commit when the code has been reformatted at the same time is a PITA. Your diff tools are useless if every line in the file has changed. –  Dave Kirby Jul 13 '11 at 22:27
add comment

Yes. Please "fix" the code as you see fit. Just like the Pragmatic Programmers say in their book The Pragmatic Programmer, no broken windows. If the code is not up to par, I consider it a broken window.

share|improve this answer
add comment

I would not have any problem with it and would probably appreciate it... as long as the changes are not "religous". Please do not go through all of my classes and move the curly braces to the first line of the method. If the formatting is a legitimate "different strokes for different folks" type thing, then it is a bit annoying when someone goes in and imposes a format on the code you most frequently edit. However, if you become the primary editor of that particular module, then make whatever formatting changes you see fit.

share|improve this answer
add comment

There are various repositories that will automatically do reformatting on check-in as well as small things like change CR/LF pairing on get depending on the platform getting the source.

There is a huge drawback to doing your own reformats in that your check in deltas will be obfuscated by tons of reformatting and if there's a regression problem it becomes more difficult to find the offending code blocks.

You might suggest to your lead that since the code base is old it should be brought in from the cold and reformatted to current standards all in one go, leading to a bright new future for code everywhere.

share|improve this answer
add comment

Since you are talking a purely "formatting" issue (meaning we aren't fixing bugs but making it look pretty to your own standard), I think it depends if the original person is still maintaining the code or not.

If the originator is still working on the project - it is rude. What might "look" right to you isn't what will "look" right to them and to modifying the code for the sake of formatting isn't polite. It can also waste lots of time.

I was working on a project once with a VERY possessive developer. Over the years, I have developed a very methodical way of formatting my code that I feel is easy to read, less prone to implicit mistakes, and self-documenting. This guy, on the other hand, favored using all implicit features with long lines which spread out 300 characters wide so you had to have a 30" monitor to read it because he believed line count was more important than readability. He spent half a day blowing through my code changing it to his "preferred standard"...while I was still developing in parallel! I came in the next morning to find two days worth of work formatted to his mess. It was rude and a waste of time.

Now if the developer is gone and you have a "better style" go for it.

share|improve this answer
add comment

Always autoformat the code if your IDE can do it.

  • Prevents manual formatting changes from cluttering your version history in the long run
  • Formatter profile has to be agreed upon amongst all the developers (pick the default?-)
  • Make formatting code and organizing imports a habit when saving a file

For example in eclipse, you can first run the formatter and organize imports for the whole code base. Then remember to ctrl+alt+f before you save.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.