15

A developer writes if/else blocks with one-line code statements like:

if (condition)
   // Do this one-line code
else
   // Do this one-line code

Another uses curly braces for all of them:

if (condition) 
{
   // Do this one-line code
}
else
{
   // Do this one-line code
}

A developer first instantiates an object, then uses it:

HelperClass helper = new HelperClass();
helper.DoSomething();

Another developer instantiates and uses the object in one line:

new HelperClass().DoSomething();

A developer is more easy with arrays, and for loops:

string[] ordinals = new string[] {'First', 'Second', 'Third'};
for (i = 0; i < ordinals.Length; i++)
{
    // Do something
}

Another writes:

List<string> ordinals = new List<string>() {'First', 'Second', 'Third'};
foreach (string ordinal in ordinals)
{
    // Do something
}

I'm sure that you know what I'm talking about. I call it coding style (cause I don't know what it's called). But whatever we call it, is it good or bad? Does encouraging it have an effect of higher productivity of developers? Should we ask developers to try to write code the way we tell them, so to make the whole system become style-consistent?

2
  • 1
    I was actually coming here to ask a very similar question just now - when automatic code formatting tools are available and convenient, is it more important for those tools to not be run (and preserve inconsistent personal styles) or to have a style enforced?
    – fluffy
    Sep 13, 2011 at 22:21
  • Consistent style makes reading code easier, therefore use a consistent style. In your examples, the easiest to read is 2, 1, 2. Though the last case is different. In performance critical applications the for loop is faster when iterating over lists. Performance is identical iterating over arrays. And in all cases, use var instead of the fully qualified type name.
    – Stephen
    Feb 8, 2017 at 1:13

14 Answers 14

19

A coding standards document is useful. It's most useful when it's short enough that anybody can remember the whole thing without too much trouble and when it doesn't cause anybody too much pain.

How you choose to indent code in your organization, or capitalize names, or implement your loops, or comment your code doesn't matter all that much; the helpful part is getting everybody to write code that looks about the same as everybody else's.

  • It avoids having to spend a minute recalibrating your expectation of where braces should be and such every single time you look at someone else's code.
  • It avoids having several different styles of code all in the same file.
  • Perhaps most importantly, having a written standard avoids arguments about coding practices during code reviews.

Again, what the standards are doesn't matter as much as having some sort of simple, straightforward standard. So, put all your developers in a room and let them argue about what the standards should be. This meeting could go on indefinitely, so the rules are:

  • Anything not decided by the end of the meeting will be decided by the manager.
  • The meeting will end after two hours, or when someone starts shouting or crying, whichever comes first.
  • The entire standard will fit (in reasonable type size!) on a sheet or two of paper, double-sided only if absolutely necessary.

Consider adopting somebody | else's | standards either as a starting point for your own coding standards meeting, or as a way of avoiding the meeting entirely.

Once agreed upon, developers should be able to (and should be expected to) police themselves. Occasional deviation from the standard shouldn't be a big deal (and might even be justifiable), but curmudgeonly refusal to abandon some favorite personal style in favor of the standard should result in immediate relocation to the office with the leaky water pipes, or whatever.

Demian Brecht points to lint tools. These are a perfect complement to a coding standards document. It's merely good to stick to coding style standards; it's important to stick to coding standards that relate to dangerous practices. Nobody other than the author is going to check that every line of code meets the standard for style, but you should certainly consider building a lint tool into your team's workflow to automatically catch likely bugs. In addition, the tool itself can codify the accepted practices so that you don't have to list them all individually in the coding standards; just specify the tool's configuration.

Note: The "coding standards" idea is not unique to programming. "Coding standards" are used in many fields, sometimes within an organization, more often across an entire industry or profession. Some examples:

In each case (and many others) a competent practitioner could easily understand "code" that doesn't meet the expected standard. Why do so many industries persist in writing detailed requirements for documents that don't even need to be parsed by a compiler? Because style matters. Presenting information in a standard style lets the reader focus entirely on content, makes reading faster and aids understanding, and reduces errors.

5
  • 1
    If you add the usage of linting tools to your post, I'll delete mine and +1 yours :) Sep 13, 2011 at 21:30
  • @DemianBrecht, Good suggestion, thanks. Added lint tools to improve my answer, but I think you should leave yours as well.
    – Caleb
    Sep 13, 2011 at 22:13
  • Well fine then. +1 anyway :) Sep 13, 2011 at 22:18
  • And also to you!
    – Caleb
    Sep 13, 2011 at 22:24
  • +1 for the "... immediate relocation ..." as in my experience such behavior is consistent with sociopath and/or narcissistic tendencies, incompatible with software development teams.
    – mattnz
    Sep 14, 2011 at 2:09
16

Style doesn't matter.

There, I said it.

After 30 years, and hundreds of client sites, and code from (estimating) 500 (or more) team members over those years, I've found that style just doesn't matter.

Get it to work, first.

Get it to use an optimal volume of resources (i.e., right data structure, right algorithm).

Fuss over "style" after everything else has been solved. Fuss of "style" only with tools, never manually.

19
  • 2
    Amen! Coding standards need to be justified in terms of real benefits and consistency isn't a real benefit, but a luxury.
    – JohnFx
    Sep 13, 2011 at 19:10
  • 12
    But style is rarely about style. It is about avoiding common errors. Sep 13, 2011 at 20:15
  • 6
    But it does help avoid '=' instead of '==' (don't use assignment inside the condition). It helps avoid if (t) { x;y;} rather than if (t) x;y; (always use '{}' after if). Prefer const correct code (it is very hard to add const correctness into code after it has been written. Use std::cout/std::cin not fprintf/scanf the former are much more likely to cause errors. Use vector rather than arrays (to help prevent overflow). Sep 13, 2011 at 21:05
  • 5
    Getting it to work is obviously paramount. But just because it works doesn't mean it's done. It also needs to be tested. And then reviewed. That all might take a few days. Then, for years afterward, it has to be read and understood and maintained. Useful code (why write any other kind?) will be read many more times than it's written, so making it easy to read and understand improves productivity far into the future. Using a consistent style across the organization is one way to help in that respect.
    – Caleb
    Sep 13, 2011 at 22:21
  • 1
    What about enforcing the use of automatic formatting tools? Eclipse and XCode make it very easy to keep code formatted - but one of the engineers I work with gets extremely upset if someone autoformats "his" (i.e. the company's) code to keep it consistent with the rest of the codebase. Formatting is only a small part of style but it's also an important one, especially for issues like if/else nesting and overall code flow (not to mention readability).
    – fluffy
    Sep 13, 2011 at 22:26
4

There are coding styles and there are coding smells. It wasn't once that I have found:

    if(debugCondition)
//         printf("DEBUG: state:%s at debugCondition\n",dbg_state());

    for(i=0; i<MAX;i++)
    {
       //some essential business logic
    }

My previous employer has rigoristically forbidden using multiline if() without braces. It was considered a "do it again and you're fired" type fault. The allowed constructs were

     if(condition) break; 
     if(condition) continue; 
     if(condition) return; 
     if(condition) for()...; 

This resulted from an error just like the one I showed above, which took a couple hours to find bringing the portal's main page to a stop.

There are things you just ought to always do. Like switch():

     case 1:
         something();
     break;
     case 2:
         something();
     return;
     case 3:
         something();
     continue;
     case 4:
     case 5:
         something();
     break;
     case 6:
         something();
     //fallthrough;
     case 7:
     ...

Meanwhile, typing:

     case 1:
         something();
     case 2:
         something();
     break;

without closing a case with //fallthrough is considered a bug.

Generally, there is the coding standard which is the guidelines which can be ignored, interpreted creatively or modified for given needs. And there's the section on "smells" and it's to be obeyed rigorously.

3

Create consistency with a good upfront design. What you describe in the question is nothing less than micro-management. I do a lot of web development. So I favor RESTful and CRUD development exposed as services. This ensures simple, well defined entities that can be used in a variety of ways.

This design also allows me to delegate implementation details to other developers. This way they are filling in the blanks rather than creating a whole system design.

A lack of an overall design creates system inconsistencies. Critique of basic iterations and looping constructs does little to improve system design. These kinds of problems are better solved with a hands off approach, StyleCop.

Can you draw a relatively simple diagram of your overall system design? A design describing the elements/entities involved in a to a new team developer. If you cannot, or it is highly involved then the design is lacking.

3

IMHO it depends...

Your first two examples are not just a matter of personal taste to me.

if (condition)
   // Do this one-line code
else
   // Do this one-line code

(without brackets) = more bug-prone, if more code is added later on:

if (condition)
   // Do this one-line code
else
   // Do this one-line code
   // ...and this line as well... oops! this will actually execute either way

And this is less convenient to debug:

new HelperClass().DoSomething(GetSomeData()); // etc.

You cannot simply set a breakpoint where you need it. If this is a one-off - fair enough, but we discuss it as a matter of style, and once you've got stuff like this all over the place, debugging gets more unpleasant than it has to be.

As for your third example (for vs foreach), I see it as a matter of taste though.

2

Neither. I would avoid harsh coding rules to avoid being tedious, nit-picky, micro managerial and worst of all wasting time. Your sense of autonomy is your own issue. If I want you to feel better about yourself, I'll buy you a round of your favorite beverage. Other than that, get something done.

2

Use of conventions, variable names, class names etc. is good practice. But coding styles like the examples you provide are rather trivial and have no effect on either the readability or efficiency of the code. On the other hand, the overhead from enforcing a given style combined with the developers effort to following it will cause problems.

2

Use an industry standard linting tool (apparently FxCop for C#). While it's not the end-all, be-all, consistency is important in large projects that have a number of people working on them.

If you're just working on your own project or small team, many would argue that it's overkill. I believe otherwise (I use PEP8 for Python, even in my personal single-developer projects).

Most of the examples you gave, however, would most likely not be caught by a linting tool as they simply don't matter.

1

Enforcing uniform coding style is always hard. Ideally such mundane task should be left to a tool to handle. I applaud the Go language community for doing that.

1

It's a mixture of both. Just because it's a standard doesn't make it readable and maintainable. If there is no instilled standard yet, or it there are faulty and unreadable aspects of it, revision is appropriate.

1

In the final analysis, it's the programmer that drives the programming. Style issues exist, but they are secondary to getting the program out.

It IS helpful to give programmers SOME style guidance. This can/should be done before the programmers are put to work, especially if they are relative beginners to the environment, or to programming itself. Given guidance, some programmers will be very style conscious, others less so. The guidance given at the beginning of the project will help the first group of programmers, thereby making the overall task easier. It might do little for the second group, which will (hopefully) make up in creativity, what they lack in conformity.

1

I think it comes down to the size of the project, and how many people are working on it. A skilled programmer can look at both differnt format styles and know what was going on in both of them but there needs to be a consictancy so the eye can catch it easily. If your going to do your single line ifstatments with out the curle braces then that project should not use them Who ever is leading that project should have that choice. I like things to look clear cut and also very compact as i can get them. If your going to make an a single line if statmeents they better looks like this

if() {}
else() {}

Or even:

if() {} else () {}

But a single line if should not use the spacing of a mulitiline if. It is just how I do things. I dont mind the way the object is called and that depends on how many times its called. if its getting called a few times then i rather start the object and depends if it has constuctors and what not also. Because if you have a constructor and you call:

Object.method();
Object.method2();

Then you ended up call the the object constuctor method twice when it could have been avoided by instancying the object fully.

When it comes to foreach and for loops Its also about the lanauge some lanauges give you better control when looking at the array in a foreach loop so you have the key and the value in temp arrays. And I know some lanagues have some sort of optimization because they can look at the loop and know exactly how many times its going to get called.

1

While your first example is not great (the argument that it's more bug-prone under modification carries some weight), in general I've actually grown to prefer that developers use slightly different coding styles.

After working with other team members' code, sometimes for as little as a few months, it essentially starts to work line an in-line git blame. After having been at my company for 10+ years, I can probably tell you with 80%+ accuracy who wrote any given class anywhere in our 500k lines of code, just by looking at it.

While there is a tiny bit of "ramp time" when you start looking at code that uses a style you don't agree with, what you're really doing in that time is saying "oh, this looks like John Doe's code (because he uses style X but not style Y, etc.)". And that brings with it a whole heap of context. To a first approximation, you know what to expect from John's code -- what other parts of the codebase he understands well and what parts he doesn't, whether he's an SQL guru or a GUI novice, etc. etc.

Running everyone's code through a formatter essentially strips out this information, hiding it behind a git blame, which you may not bother to run.

0

This really depends a lot on the type of organization.

If you're a small group of superheroes, then any type of style is fine. If everyone has their own style, and it's both good and internally consistent, then that's all the process you need. The Superheroes will say, "Jane has brackets, so this is what she means" or "Jack uses camelCase for variables".

Universal standards tend to be best for making everyone a B+ player. Your superheroes will get pulled down by this. But if you want one A player and a half dozen B players and a half dozen C players to average up to the B+, then you want consistent standards. In this instance, you want everyone doing things the same so that the one A player can walk through everyone's code as quickly as possible.

Many of you way, "But wait, why not do away with the B and C players?"

The reality is there aren't many Superheroes. While it may pay to find and nurture them at Google, putting in a new billing system for Ma Bell may be more cost effective with the latter team. (And if you really have Superheroes, 4 or 5 of them will be twice as expensive as a dozen juniors)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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