Re: On Coding Standards and Code Reviews
by ELISHEVA (Prior) on Feb 19, 2009 at 09:41 UTC
|
My first impression is that "whew! you've put a lot of work into this." My second impression is that your work has just begun.
The list is too long and the potential conflicts between some of the various standards you've cited need to be explored. So too, the match between the list and the problem areas for your particular team. The key is focus: too much information is equivalent to no information at all.
Conflicts between "best practices" isn't necessarily bad: good architecture is often a delicate balance between competing principles (e.g. abstraction and simplicity). In fact, discussing and exploring competing principles as a team is a great way to develop good programming judgment about which principles should apply when.
I also noticed that nowhere in your list of sources did you mention interviews with your team members. There is another thought you might want to consider that came up in a recent discussion between tilly and myself: mentorship. Programming is as much an art as a science and if there are experienced members of your team with proven design and implementation skills, it is a good idea to ask them for their insights and to encourage them to coach the less (software wise) mature members of the team. Sometimes the bluebird of happiness is flying around in your own backyard. A design principle taught and tied to the experience of an old hand and a member of the team is going to carry a lot more weight than the rhetorical pronouncements of the latest gurus.
Best of luck with this project, beth
P.S. If you have a manager that doesn't get that last bit (rhetoric vs. ...) I have a nice article from Harvard Business Review on the Rhetoric of Action that I can dig up for you - and I'm sure I can find others as well. Just msg me. | [reply] |
Re: On Coding Standards and Code Reviews
by Tanktalus (Canon) on Feb 19, 2009 at 05:57 UTC
|
I guess it may be just me, but I'm curious as to why. Some of your suggestions/commandments are purely arbitrary, and I don't use that in a negative light as sometimes there is no best choice and consistency is more important, but I'm still curious as to why you chose that arbitrary rule over another similarly-weighted controversial rule. Just to pick on the first one that popped out, using spaces over tabs. Don't get me wrong, I agree with it, but I know it's contentious, and would prefer to see something that expanded on why. This goes doubly, if not more, if you're going to need to not just defend your rules against peers in your organisation, but against every single new hire for the rest of your tenure there. It also allows the rules to be properly challenged if, for example, the significance of your decision is reduced. As an example, continuing to pick on the spaces vs tabs, perhaps it is desirable for each developer to use the tools they're most comfortable with. That may mean some of your developers using eclipse, others using emacs, others using vi, and the odd wacko using notepad, as long as each is highly productive. But then, perhaps your management (either personnel or technical) decides for some reason to standardise on an environment - whether that's MS Visual Studio, Eclipse, or Komodo. Now maybe the reason for spaces goes away, and it can be revisited. But without a continued, on-going understanding of the reasoning, your rules become unchallengeable shackles or cargo cult, possibly long outliving their usefulness. I know you have lots of links, but, without going through each one manually, it's difficult to know (in general) which link defends which rule. Some are obvious, but not all, so you should still have as direct of links as possible for most items.
My second suggestion would be to store the whole thing in a corporate wiki where others can add their reasons for (or possibly even against) each dictum, making it (hopefully) feel less like dictum, and more like group selection, and thus easier to self-enforce. It also would help keep the document living which helps it stay in use, as well as keeps it available for challenge.
| [reply] |
|
Just to pick on the first one that popped out, using spaces over tabs. Don't get me wrong, I agree with it, but I know it's contentious, and would prefer to see something that expanded on why.
This is discussed in Perl Best Practices, page 20, "Indent with spaces, not tabs". Quoting Conway:
Tabs are a bad choice for indenting code, even if you set your editor's tabspacing to four columns. Tabs do not appear the same when printed on different output devices, or pasted into a word-processor document, or even just viewed in someone else's differently tabspaced editor.
Not being an "expert book author", many folks here won't listen to me, so I prefer to point them to a well-respected reference, such as PBP.
For some of these layout rules (e.g. brace placement), we already fought a war some years ago and I don't want to reopen old wounds. With over ten million lines of existing code, it doesn't seem worthwhile to change longstanding company code layout conventions.
I know you have lots of links, but, without going through each one manually, it's difficult to know (in general) which link defends which rule. Some are obvious, but not all, so you should still have as direct of links as possible for most items.
Agreed. I'll try to provide a reference/s for each rule. I expect that will prove to be important for some folks here who may object to blindly accepting a rule without a supporting reference.
| [reply] |
|
Tabs are a bad choice for indenting code, even if you set your editor's tabspacing to four columns. Tabs do not appear the same when printed on different output devices, or pasted into a word-processor document, or even just viewed in someone else's differently tabspaced editor.
Having lot of respect for Damian Conway I would like to point out that almost all useful editors allow you to define the tab length. Be it 2, 4 or 8 spaces. Using tabs allows every developer to have their own indentation level by defining the tab width in their preferred editor/pager/viewer settings. Using spaces forces one indentation scheme (and forcing people is a bad thing). I always tell people who are arguing over indentation that whether you choose to use tabs or spaces you must use it systematically. Do not mix tabs with spaces.
I use spaces my self (spaces are what I am accustomed to), but I do not despise tabs.
--
seek $her, $from, $everywhere if exists $true{love};
| [reply] [d/l] |
|
|
|
|
|
|
For Pete's sake people it's tabs and spaces!
How many hours of productive time has been wasted in meetings arguing the pros and cons of each? Does Perl care whether you've used spaces over tabs or tabs over spaces? We've already agreed that most editors can seamlessly convert between tabs and spaces of any desired indent size. The pettiness of it just blows my mind.
Here's a standard: forget standards. Use Perltidy.
A large portion (if not all) of the coding standards listed here could be configured into Perltidy. If every piece of code passed/used/saved anywhere in the company is enforced through Perltidy, then all code will conform to a single layout standard. The coders don't need standards, they can code to what ever style they feel comfortable with. After a pass through Perltidy everybody's code will conform to the same single standard.
In time coders will make an effort to conform to that same standard that they've been immersed in because it's just easier to adjust one's personal style choices than to buck miles of existing code. But even if they don't come around, who cares? In the end the code will come out formatted to standards regardless, and all the time that would have been wasted arguing over standards can be put to better use getting actual work done.
end grump
| [reply] |
|
|
Re: On Coding Standards and Code Reviews
by dHarry (Abbot) on Feb 19, 2009 at 10:09 UTC
|
I thank you for the effort and up-voted you accordingly! I do have some remarks though.
Most of the general guidelines are too general to be of any use. How are you going to address them? What do you need to do when, and how are you going verify they have been met?
Take for example Robustness, how to address that?
What I miss is the Software Development Life Cycle, the process. You need to make the abstract statements tangible and embed them in a process. In my experience this is the only way to make it work, especially when projects get bigger and/or the SW has a zero fault tolerance architecture. i.e. SW for a nuclear plant, medical applications etc.
Also where does architecture comes in? What I mean is the general guidelines may conflict with each other. Then you will have to determine what is more important, likely on a per project basis. I like to view things like Robustness, Efficiency, Maintainability, Scalability, etc as “Quality Attributes” in the architecture and give them weights depending on their importance (depending on the specific project). With a simple excel sheet you can make a more founded decision on what to focus on. There are always restrictions (budget, time,...) and chances are you can’t do all things in your list. Focusing on the important ones and doing them good makes more sense then doing all of them but half.
I've been asked to prepare some guidelines on coding standards and code reviews
Coding Standards and Code reviews are typically instruments of the (much) wider area of Software Quality Assurance. The code reviews they fall in the SQA activity category SW verification/validation.
I don’t know the size of your company but I assume it must have at least one QA guy, maybe even a full-blown QA department. In any case QA people need to be involved in these type of activities.
HTH
| [reply] |
|
Thanks for the feedback. This is the sort of feedback I was hoping for, so I can be prepared before I meet with our QA guy. :-) To answer your question, we have one QA guy and I'll need to refine these notes before I meet with him. As you might expect, he's more interested in process than I am, his primary concerns being things like "how should we enforce the coding standard?". In the past, we've sorta had coding standards (actually, too many of them) and informal code reviews, but they've never been enforced consistently across the organisation, especially when deadlines are tight.
| [reply] |
|
One way to enforce code quality and integrity in the process is to choose a process model that can facilitate it. Agile methods are good at it. Especially I would recommend looking at Scrum and pay attention to the Definition of Done.
A self guiding process is the best tool to get teams happy, efficient and committed. This includes code quality and standards. I would not dictate them except on a very high level (setting some thresholds). IMO the tools do not matter that much, the result is what I would be interested in. And a good process that includes tools for correcting bad ways of working is priceless.
--
seek $her, $from, $everywhere if exists $true{love};
| [reply] [d/l] |
Re: On Coding Standards and Code Reviews
by gwadej (Chaplain) on Feb 19, 2009 at 15:00 UTC
|
Like many others, I'm very impressed with what you've written. I see it as a survey of the things we might possibly want to consider when doing reviews and such.
I recently worked in an organization which had a requirement of tool-assisted review for every piece of code that went into version control. I can say for certain that a much smaller list would be more useful for focusing reviews.
Picking a single coding standard that everyone can work with is a very good idea. Separate from that a (short) checklist of things to be aware of when reviewing is helpful.
Were I beginning this kind of a process (again), I'd probably start with your list and start paring them down. Anything that everyone does already (or strongly agrees with) does not really need to be on the checklist. Anything that everyone agrees with and tries to do should be on the list. Start with the low-hanging fruit. What gives the most advantage with the least disagreement?
In my experience, the bigger the list the more likely to generate one of two reactions:
- "That's nice." and everyone ignores it
- "I disagree with this one little nit, so I won't do it."
Generating a shorter checklist from your overall list is less likely to trigger either of these reactions.
The great thing about your list, for me at least, is that it gives a broad overview from which someone could pick what is important for their group.
| [reply] |
Re: On Coding Standards and Code Reviews
by CountZero (Bishop) on Feb 19, 2009 at 17:04 UTC
|
Wow, impressive compilation of standards! ++ already just for that.My view on standards, whether for programming or any other team-work related matter, is that you must have some clear and well-understood principles which will be enforced in any case. If it has exceptions it should not go in the standards. The standard may be controversial, but once someone (with the power to do so) decided it is the standard, there is no discussion and the team follows it or you get out of the team. Of course that means that the team members must be able to follow the standards and it is my experience, if you have more than 5 principles/rules/standards to follow, you are too likely to miss one and people will understand/accept that you missed one. That is a slippery slope and before you know it everyone will do again as they did before and not follow any of the rules. So pick five (three is even better) practical and concrete principles, rules, standards, best practices, ... post them in a conspicuous place and rigidly enforce them. Things like "correctness comes first" is not a good principle: if it is not correct it is wrong and it simply doesn't count. This is a meta-principle or pre-condition or part of the environment and comes before you even consider your rules. "Robustness, Efficiency, Maintainability" is too vague and cannot be enforced. "Benchmark before you optimize", "Log all errors" and "The result of every file operation or API call must be checked, and unexpected results handled." on the other hand are examples of good standards and are easy to maintain and enforce. If you can settle on five general rules, maybe you can add three for each of the different languages. Lay-out should be entrusted to a code-prettifyer such as Perl-Tidy or similar, rather than put this in a rule-set. Then everybody can code as they please and before they check in their code it gets transformed into whatever form or layout is required. Some code-repositories can even do that automatically. If you cannot entrust it to a code-prettifyer, forget it. Do you really want to check the lay-out of someone elses code for the rest of your career?
CountZero A program should be light and agile, its subroutines connected like a string of pearls. The spirit and intent of the program should be retained throughout. There should be neither too little or too much, neither needless loops nor useless variables, neither lack of structure nor overwhelming rigidity." - The Tao of Programming, 4.1 - Geoffrey James
| [reply] |
Re: On Coding Standards and Code Reviews
by salva (Canon) on Feb 19, 2009 at 08:46 UTC
|
1. Robustness, Efficiency, Maintainability.
Wrong! for most applications efficiency* should not be a priority anymore... at least not your second top priority.
And BTW, correctness is the first one!
(* efficiency understood as getting your application to use as few resources as possible, not as programmer efficiency or any other thing)
| [reply] |
|
| [reply] |
|
I think this brings up another good point. Put your rules in order. Anyone reading them will find the first rule that seems to apply and stop reading. If something is most important, put it first. There will always be conflicts (speed of app vs speed of development), and it is important to put them in the order which you want them to supercede each other. This is not supposed to be a mystery novel, building suspense. Even then, there will sometimes still be times where you can't follow the rules - what then? Obviously, a senior developer (or just a prick like me) will ignore rules that don't apply in a specific circumstance (perhaps you have something running during mouse movement - it obviously has to be fast, or the whole system will die, even at the expense of readability of code). Perhaps a guideline on how to determine you're in a special case, but, more importantly, what to do with that special case. Perhaps a comment block to highlight the non-standard code, why, and what it really does in plain English rather than the mess of code it may really be?
(++ for both of you for noticing it and bringing it up :-) )
| [reply] |
|
Re: On Coding Standards and Code Reviews
by JavaFan (Canon) on Feb 19, 2009 at 23:05 UTC
|
No long lines. Limit the line length to a maximum of 120 characters.
In my book, 120 characters is a very long line. When editing code I didn't write, I always leave the formatting as is. It usually isn't my writing style, but I can write Perl even if it doesn't have the spaces where I usually put them. I'm not at all a fan of enforcing formatting - I find that as counter productive as enforcing wearing ties or high heels.
But there's one formatting thing I change when editing code. And that's breaking lines exceeding 80 characters. I've my own set of stylistic issues (which I don't enforce, or want to enforce on anyway - they are just for myself), and each point will be broken once every blue moon for some reason. Except the 80 character line limit. That never, ever gets broken. 80 characters is the absolute limit a line may have.
| [reply] |
Re: On Coding Standards and Code Reviews
by talexb (Chancellor) on Feb 19, 2009 at 21:50 UTC
|
I like my code laid out in a particular way, but your section on Code Layout really clamps down on a particular style. I'm not sure if it's going to be worth the effort that make absolutely sure that everyone's following exactly the same format.
A better approach is somewhere between 'I like to format my code like this' and 'I'm making changes to some existing code so I'm going to modify my coding style so the new code doesn't clash too much with the old code'
For me, a code review is more about 'is it readable and maintainable?' than 'is the one correct true style being followed?'
Alex / talexb / Toronto
"Groklaw is the open-source mentality applied to legal research" ~ Linus Torvalds
| [reply] |
Re: On Coding Standards and Code Reviews
by mr_mischief (Monsignor) on Feb 19, 2009 at 19:01 UTC
|
Some of your rules seem repetitive. Perhaps you should move the ones you feel are worth repeating toward the top and have fewer rules overall. A policy has the most effect when it is actually followed. It's more easily followed if it's more easily digested and even memorized. Fewer core rules with more moved into the references and subrules sections might make it easier to follow. | [reply] |
Re: On Coding Standards and Code Reviews
by wol (Hermit) on Feb 19, 2009 at 18:08 UTC
|
Remember that code maintainability is paramount.
Here here. By definition, if the code is maintainable, you can change it to also conform to any/all of the other standards afterwards.
Of course, there's plenty to say about how to make code maintainable...
--
use JAPH;
print JAPH::asString();
| [reply] |
Re: On Coding Standards and Code Reviews
by Anonymous Monk on Mar 02, 2009 at 20:35 UTC
|
I would probably add writing unit tests for the code you have written or going to refactor (if there are no unit tests written earlier).
| [reply] |
|
| [reply] |
Re: On Coding Standards and Code Reviews
by ack (Deacon) on Mar 02, 2009 at 18:42 UTC
|
Wow! Great node. Lots of ideas and comments; all great! This node happens to hit home for me right now. I've been given the task of developing and intitutionalizing a set of System Engineering "Best Practices" for our work developing and fielding a variety of systems and I am facing many, many of the exact same issues as brought up here...though mine pertain to overall engineering rather than coding. I have found many of the ideas and comments on this node surprisingly meaningful and applicable to my challenge and will weight all the comments carefully to find a multitude of nuggets. Some ever spur me to think about adding some coding-specific "best practices" for our systems' software, too. PM always delights, astounds, and teaches so much to so many. Thanks to you all!
| [reply] |