views:

574

answers:

18

I am working on a project which other developers work on. I would like the code to be standardized. I don't necessarily care what standard it is (K&R, GNU, 2 lines max, 1 line max, spacing between commas, etc..) just that it is consistent.

I was thinking, that as a separate checkin, I could run a beautifier on the source code.

What do you think? As a developer who works on code with other people, if somebody else decided to run a beautifier on code you work with, would you be upset?

This is code that has been written by a lot of developers over the past few years, some of them gone, some not.

Obviously, I would approach everyone with this idea first, but I am curious what SO thinks.

EDIT: I would most likely look to do this at a milestone or after a release.

+27  A: 

I think one of the major considerations should be "what impact will this have on the ability to diff?"

gahooa
Do diff tools in build systems allow you to skip a checkin? i.e.; can you diff 1 -> 3 directly without including 2?
esac
Ask that question on the gcc mailing list ;-)
Richard Pennington
esac, sure but consider what that would (or wouldn't) actually buy you. Version 3 will effectively include the changes of version 2 (presumably the formatting changes), so it may actually end up being harder to interpret, not easier.
Logan Capaldo
+1, changing that much of the code is likely going to introduce more maintenance problems than in solves. If you ever have to merge something from before, you're going to have phantom conflicts.
Chase Seibert
+1  A: 

I'd probably be upset. I have a tendency of dropping blocks of text on top of complicated pieces of code so that I can come back and easily figure out what's going on without actually reading the code. Most formatters tend to destroy that.

Malaxeur
Just curious what you mean by blocks of text? Comments would be left in their current locations.
esac
+5  A: 

I wouldn't mind, so long as any updates are checked in with the agreed format.

OMG Ponies
+6  A: 

Don't you have somebody in charge who makes decisions on coding standards? Rather than taking it upon yourself to do something like this, you should ask that person about it.

paul
Standards are for future versions of code, not current. When you look at the code, you see about 15 different 'standards' in place :(
esac
I understand, the future is what I meant, and I understand it's frustrating to read code like that. But if the guy in charge doesn't like it, he'll just go back to the way it was, or just let it gradually become a mess again...in the future. Plus he'll be be annoyed at you. In the future. Talking to your team members is fine, but if nobody's going to enforce it (in the future) it doesn't matter one bit.
paul
+4  A: 

You'll need a decent level of buy-in from the current team.

Do you have unit tests for this code? Make sure they pass before you check in. If not ...

If you have any kind of change control for where this code gets delivered too you may generate a boring admin headache - "so why did these 5000 files change again?"

martin clayton
All changes, even just formatting, need to pass unit tests, so yes :)
esac
+8  A: 
8    8 8"""" 8""""8 
8    8 8     8      
8eeee8 8eeee 8eeeee 
  88   88        88 
  88   88    e   88 
  88   88eee 8eee88
Eduardo Molteni
won't downvote, but I'm seriously annoyed by that loose "e" in the S.
ldigas
Ahh, the mysteries of the WMD control :)
Eduardo Molteni
+5  A: 

If they're "offended" by this, they're doing it (life) wrong. Annoyed, on the other hand, is completely understandable.

This is something that needs to be done separately by the developers currently working on a given part of the codebase. Doing the whole codebase at once will screw up diffs and cause merging headaches.

On a large project, just doing it after a "milestone" or "release" is not going to be enough. There's an excellent chance that some of the developers have stuff they're sitting on, just waiting to move it into the trunk after the milestone/release is branched or tagged.

Nicholas Knight
+2  A: 

With two exceptions, I would be somewhat upset, unless the code was in really bad shape. A competent programmer should not make their code unduly messy in the first place, and if it is it should be fixed after discussion with other human beings. Massive reformatting just tends to add noise to the change history. It's hard to diff and it's hard to tell whether anything important was changed.

The exceptions are:

  • cleanup of tabs vs. spaces, if this is done appropriately for the source language.
  • cleanup of trailing whitespace (less of a problem, but I have a psychological aversion to it)

Sometimes people ask for ways to get their source control system do things like this automatically. It can copy with newline conversion, why can't it clean the rest of the code up a bit too? The answer I've heard is that anything more is a surprising change that the committer cannot easily predict and hence should not be responsible for causing.

Edmund
+2  A: 

The answer depends on several factors.

1.) How confident am I in my own abilities and code? I'd like to think that I can be open-minded and learn something from it, but in a real-life situation, would I actually be able to do that? That, in turn, depends on..

2.) What are your intentions? And I'm not referring to your actual intentions, but rather my impression of them. Are you out to "fix" everyone's code? Make everyone's code consistent? Something else? If I get that you have good intentions, I'm more likely to be agreeable. If I think you're just being arrogant, I'll resist it.

and, of course..

3.) What kind of code beautification are we talking about? How extensive is it? Something that makes major changes is harder to accept than modifying indentation here and there.

steve
+2  A: 

In our code base there are a few different styles present. We've stanardized on something now for new code. For old code we either follow the style in the file if the changes are small or if doing an overhaul/major refactoring of files bring them up to date with the new standard. Really if the older files aren't being modified much and no human is looking at them, who cares what they look like. Really the benifit of standard looking code is for when humans are looking at it (code reviews, etc).

Jeremy Raymond
+1  A: 

I beautify other people's code by hand all the time. I don't see anything wrong with it as you're making it more readable, not just formatting it to your preferred method.

Bloudermilk
+2  A: 

If you have three developers in a code shop, you can probably get four different coding standards. I think beautifying the code can work, but don't waste time on religious wars deciding what standard to use.

In my experience supporting and extending a code base for 15+ years I found the following useful:

  1. You probably won't be able to get agreement on more than about 2 or 3 really basic things in a coding standard, so be prepared to accept many variations of "OK". When it comes down to it, there is no compelling logic in any coding standard rule.
  2. Don't gratuitously rewrite code. One developer whose really old code I supported would not use C keywords like break, continue, and return, so would end up with highly indented code. When debugging and fixing his code I would rewrite those routines in the fix, and only those routines, to get a structure that I thought was much easier to understand. Going through and repeating this exercise on all the other code would have been wasteful and potentially introducing more bugs.
  3. Get over trailing spaces, spaces instead of tabs, etc. There is nothing designed to slow progress down like doing a diff between the recent code and something from about 5 releases back that has just had a bug reported on it to find differences on every line due to tab/space conversion!
  4. Don't be religious about line length limits driven by paper or screen size. Technology is always changing and today's limit may seem quaint tomorrow (80 column terminal screens?).
  5. Changing code without a compelling reason is a waste of time and potentially dangerous in terms of code quality.

And a final salutory tale. We had a header file with a large number of #defines in it. One developer decided in isolation to turn this into an enum, because it was "purer". However, this promptly broke the Web GUI builder tool that read the source code and expected to find #defines. Remember, sometimes things are the way they are for a reason...

Tony van der Peet
+3  A: 

Make code beautifier available to everyone, and show them how easy it is to auto-standardize your code before every submittion. After a while people will start to trust automatic formatting, because they will notice how much easier it is to read standardized code, and how much pain the ass it is to diff some code, that is auto-formatted rarely (because of all whitespace changes). It's just easier to auto-format every modified file once per submittion. At least in c# it works great, just need to remember to hit "ctrl+e,d" once in a while.

AareP
+1  A: 

I would feel a bit annoyed, but not really offended.

I try hard to follow that my code is its own documentation, and that it's a piece of art. It's like saying let's do some photoshop on the Mona Lisa! So, I don't think I'd do this to someone else's code.

And whenever I modified how another programmer's code looked, (not the logic or the syntax, the tabs and the spaces) I have seen them feel a bit off.

aredkid
+1  A: 

No, I wouldn't be offended if someone else "beautify" my code. Beautifying code won't affect the code's function in any significant way and it also helps standardize code to make it more readable. I installed Polystyle not too long ago and really appreciate it.

netrox
+1  A: 

Are you attacking the right problem? You could beautify/format all the code now, but unless all the other developers start naturally developing in that style, it may well need beautifying after each block of development...which I guess could be part of your development life cycle...

davidsleeps
+1  A: 

Introduce the code beautifer / style checker as a part of every developer's tool set and automate its use from the IDE. Make the team agree on a common style to use and after that make the style check a unit test ran by the daily build. If any one commits code that doesn't follow the agreed style, he will break the build.

Taneli Waltari
A: 

Don't IDEs have the option of formatting code as you want it (eg Eclipse)? Why can't you incorporate a code beautifier as part of the commit process? When you commit a bit of code, it automatically goes through the beautifier.

If individual programmers don't like the style, they can reformat it at will in their IDE.

Then again, that may just remove the need to beautify on commit...

masher