10 Commandments
-
Does that matter? Tabs should be used for indentation, spaces should be used for lining things up. The size of indentation does not matter at all. That's one of the advantages of tabs - The user can set their preferred tab size.
The age old question... does length matter... Say I start working on a large solution (hundreds of files). Since the solution uses tabs, I have no idea what the intended spacing was. I assume 8 spaces, so I set tabs to use up 8 spaces. I start commenting at the end of code lines using tabs, and they line up fine for me. Say I work on 30 files like this. Then I come across a file where the comments don't line up. I find out it's because the previous person who worked on this was using 4 spaces for their tab length. I now have to go back and change my comments or all the other comments. And the reason to use tabs in comments like that is because it's faster than holding the space key. That problem could have been solved by using spaces in place of tabs. I personally don't care though. I pretty much never comment at the end of a line of code (I prefer to comment above code). I can see advantages to using tabs too. For example, if you are reading some heavily indented code, you could temporarily set the tab size to 1 space, which would make it easier to read. Both techniques have benefits, though the benefits of each are rarely applicable (at least, for me).
-
I have been asked to work on our Code Review Procedure. This sort of thing: Pascal casing used for type and method names Camel casing used for local variable names and method arguments ... A single file should only contribute types to a single namespace. Avoid having multiple namespaces in the same file. Avoid files with more than 500 lines (excluding machine-generated code). Avoid methods with more than 25 lines. ... Never hard-code a numeric value, always declare a constant instead. ... I have the feeling it is missing what really make maintenance go bad. Things like re use existing classes, don't duplicate code, separate logic, data access, and display. Can anyone point me to some documents with good recommendations of this type. The idea is to minimize defect and certainly not to alienate the programmers (including me).
If the "rule" is to "always declare a constant", then there should also be rules like: "Don't make constants public" ... or ... "Always rebuild all assemblies that reference public constants in other assemblies" Constants will get compiled "inline"; there are no references. If you change a public constant in an assembly (eg. a DLL), and don't recompile the applications that use, say, just constants from that assembly, those applications will continue to use the "old" values of those constants (that were compiled into their code). (Use a readonly field instead)
-
I have been asked to work on our Code Review Procedure. This sort of thing: Pascal casing used for type and method names Camel casing used for local variable names and method arguments ... A single file should only contribute types to a single namespace. Avoid having multiple namespaces in the same file. Avoid files with more than 500 lines (excluding machine-generated code). Avoid methods with more than 25 lines. ... Never hard-code a numeric value, always declare a constant instead. ... I have the feeling it is missing what really make maintenance go bad. Things like re use existing classes, don't duplicate code, separate logic, data access, and display. Can anyone point me to some documents with good recommendations of this type. The idea is to minimize defect and certainly not to alienate the programmers (including me).
-
I have been asked to work on our Code Review Procedure. This sort of thing: Pascal casing used for type and method names Camel casing used for local variable names and method arguments ... A single file should only contribute types to a single namespace. Avoid having multiple namespaces in the same file. Avoid files with more than 500 lines (excluding machine-generated code). Avoid methods with more than 25 lines. ... Never hard-code a numeric value, always declare a constant instead. ... I have the feeling it is missing what really make maintenance go bad. Things like re use existing classes, don't duplicate code, separate logic, data access, and display. Can anyone point me to some documents with good recommendations of this type. The idea is to minimize defect and certainly not to alienate the programmers (including me).
If it's in C# then the standard MS guidelines are pretty good as far as coding standards go but in terms of a review we have a slightly higher level checklist as well which includes: 1) Have all aspects of the design been met 2) Is all of the code relevant to the design 3) Does the code meet the coding standards 4) Does the code clearly express its intention 5) Has the code been written without duplication 6) Is the code written using the fewest number of classes and methods It's really all about checking for duplicate and superfluous code as well as making sure that the code is clear and concise and well written, it also encourages self-documenting code. Another trick we try to use is to make the compiler do as much of the work for us as possible, so tell it to raise warnings as errors etc... also use coding standards such as placing literal values on the left of an equality operator, e.g.
if (0 == myValue) { }That way if an assignment operator is used instead of an equality one then the compiler will warn you about this as you can't assign a value to a literal. Used the other way around its a bug because an assignment will return a result of true and so the code will compile and run quite happily. -
Pascal Ganaye wrote:
This sort of thing: Pascal casing used for type and method names Camel casing used for local variable names and method arguments
This sort of things is best checked automatically by something like FxCop. Code reviews are about sharing knowledge and catching logic errors.
100% percent agree. The other advantage of code reviews is that they force you to code with the knowledge that you're going to have to justify what you write to someone else.
-
Then all is fine. Oh, another advantage of symbols is they help you search for things. If FIRST_DAY_OF_WEEK was not used but its value were 1, searching for 1 would hit a lot of things you're not looking for, whereas searching FIRST_DAY_OF_WEEK would only hit relevant instances. :)
Luc Pattyn [Forum Guidelines] [Why QA sucks] [My Articles]
Prolific encyclopedia fixture proof-reader browser patron addict?
We all depend on the beast below.
Snide comment from an OO zealot: So what's FIRST_DAY_OF_WEEK * FIRST_DAY_OF_WEEK then? </snide>
-
Pascal Ganaye wrote:
Never hard-code a numeric value, always declare a constant instead
Be careful with this one; sometimes you get morons who do this:
const int IntZero = 0;
const double DblZero = 0.0;
const int IntOne = 1;
const double DblOne = 1.0;
const int IntTwo = 2;
...Software Zen:
delete this;First rule: there is no cure for stupidity. Second rule: code review is meant to catch this type of stuff and figure out who the management problems are, and who needs training. If you are doing some level of review on a regular basis, you'll figure out who needs 'attention'. Rather than set an arbitrary set of lines of code for a method, I normally go by a rough cyclomatic complexity calc.. just score a point for every if condition in the method.. if it gets above 6 or 7 then better breakdown may be in order. Note that I've SEEN methods 1000s of lines long that exceed my rought cyclomatic calc complexity of 100. Some folks just can't be trusted with an editor/compiler. ;) Difficulty in maintenance is usually related to these things: Too much complexity in if conditions.. very difficult to understand. Too many methods for doing something simple.. you have to jump around and remember too much just to do something simple. Variables that are used for more than one purpose. Methods that have more than one goal. Incorrect usage of threads (typically lots of threads for little purpose). Lack of usage of locking mechanisms between threads Variable names that lack meaning. Lack of encapsulation of data in OO systems (I've seen this SO much). This one is a serious bugaboo.. since anything related to changing a datum in such a program means a review of the entire (or close to entire) program. Lots of copied code. Pointer errors. Non initialized variables. (I could go on.. this is just what at the top of my priority list for reviewing my code and others..) Keep in mind the goal of maintenance: minimum changes to localized code. If you are coding in a way that makes things not local or make changes required to be widespread, something is wrong in what you are doing.
-
Why even have a main body inside the for loop? Why not:
int i = 0; while (i < 100 && ReturnTrue(new MethodInvoker(delegate { DoSomething(i++); }))) ;
:rolleyes:
-
Does that matter? Tabs should be used for indentation, spaces should be used for lining things up. The size of indentation does not matter at all. That's one of the advantages of tabs - The user can set their preferred tab size.
The problem is when different people edit the same files using varying setups - some using tabs, some using spaces, plus varying tab interpretations (4, 3, 8, etc ...). The end result is an indentation nightmare that looks as if a monkey has been dancing on your keyboard.
-
I have been asked to work on our Code Review Procedure. This sort of thing: Pascal casing used for type and method names Camel casing used for local variable names and method arguments ... A single file should only contribute types to a single namespace. Avoid having multiple namespaces in the same file. Avoid files with more than 500 lines (excluding machine-generated code). Avoid methods with more than 25 lines. ... Never hard-code a numeric value, always declare a constant instead. ... I have the feeling it is missing what really make maintenance go bad. Things like re use existing classes, don't duplicate code, separate logic, data access, and display. Can anyone point me to some documents with good recommendations of this type. The idea is to minimize defect and certainly not to alienate the programmers (including me).
-
I agree with most of them, but one that really bugs me is:
for (int i=0; i<100; i++) { DoSomething(i); }I prefer:for (int i=0; i<100; i++) DoSomething(i);or:for (int i=0; i<100; i++)
{
DoSomething(i);
}With the last one, who knows what future statements may need to be added... Another one that bugs me is stating that 4 spaces should be used instead of Tab, sorry, Tab tab tab ;P
"The clue train passed his station without stopping." - John Simmons / outlaw programmer "Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon "Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
I would always put spaces around binary operator, it's more readable. And you can use the prefix ++ operator in a for loop exactly as the postfix operator is used, but the prefix is more efficent cause it doesn't have to return the previous value, although i would assume the compiler probably optimizes this well.
Pete
-
From the Microsoft one: "Do not use a prefix for member variables (_, m_, s_, etc.). If you want to distinguish between local and member variables you should use “this.” in C# and “Me.” in VB.NET" What? I thought the .NET guidelines said to use an underscore? :wtf:
Although I agree with them (I have always disliked prefixes on member variables), I do understand where you are coming from. M$ has always been consistently inconsistent. :)
Henry Minute Do not read medical books! You could die of a misprint. - Mark Twain Girl: (staring) "Why do you need an icy cucumber?" “I want to report a fraud. The government is lying to us all.”
-
The things you have mentioned in the first part of you post are already checked by FxCop [^] and StyleCop[^] and can be automated. They may or may not directly impact the number of defects. FxCop to some extent finds certain things which might be problematic. Using StyleCop, on the other hand, you can enforce all the styling. This will prepare you for the next step which is Manual Code Review. Manual Code Review should be to catch subtle things: not protecting shared variables, potential memory leak, better way to do a certain thing (use an API instead of writing code to implement the functionality of API). Code Review can catch lot of subtle bugs and that is why it is so effective. Also, if there are many programmers in your team it is essential that you use automated tool to analyze the style and code before they sit and do a code review. This is because programmers usually get nit-picky of style and miss the more important things. If you have common auto enforceable guidelines, it helps you get to the next step.
I agree, go the whole StyleCop standard holus bolus. That way you avoid holy wars, its an arbitrary standard that Im sure you'll disagree with some parts of,b ut you will disagree with parts of any standard. After using it for a month I'm totally used to it.
-
Why even have a main body inside the for loop? Why not:
int i = 0; while (i < 100 && ReturnTrue(new MethodInvoker(delegate { DoSomething(i++); }))) ;
:rolleyes:
I used to code that way, I got tired of being the only one to be able to understand my code. In designing electronic circuits, I learned that if you want the technician to correctly build what you intend the circuits need to be drawn in their classic configurations ie: a voltage divider with the resistors one over the other, op-amps input on the left output on the right. It is the way they learned to use and understand them. The most complex code is build of simple structures. HoleyMoley
-
I agree with most of them, but one that really bugs me is:
for (int i=0; i<100; i++) { DoSomething(i); }I prefer:for (int i=0; i<100; i++) DoSomething(i);or:for (int i=0; i<100; i++)
{
DoSomething(i);
}With the last one, who knows what future statements may need to be added... Another one that bugs me is stating that 4 spaces should be used instead of Tab, sorry, Tab tab tab ;P
"The clue train passed his station without stopping." - John Simmons / outlaw programmer "Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon "Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
-
I have been asked to work on our Code Review Procedure. This sort of thing: Pascal casing used for type and method names Camel casing used for local variable names and method arguments ... A single file should only contribute types to a single namespace. Avoid having multiple namespaces in the same file. Avoid files with more than 500 lines (excluding machine-generated code). Avoid methods with more than 25 lines. ... Never hard-code a numeric value, always declare a constant instead. ... I have the feeling it is missing what really make maintenance go bad. Things like re use existing classes, don't duplicate code, separate logic, data access, and display. Can anyone point me to some documents with good recommendations of this type. The idea is to minimize defect and certainly not to alienate the programmers (including me).
I prefer to ignore all formatting arguments. I've given up on trying to enforce tab rules - every editor seems to handle them differently. Most languages have pretty-printers, and/or they're not horribly difficult to create. Here's what I have come up with for our standards (we're primarily C but some C#). Much of it is very subjective. The programmer should be allowed significant flexibility, with the most experienced, even-handed developer having the final say. Commenting: File header blocks Header blocks for functions and substantial data structures Adequate comment frequency and quantity Legible and helpful comments (perfect spelling/grammar not required) Accurate All commented-out code is worth retaining and explained Coding Style: Standard and consistent Braces around single-statement blocks (to eliminate macro-inserted logic bugs) Magic (hard-coded) numbers minimized Constant declarations used where appropriate Meaningful variable names (i is sometimes appropriate) Global and local symbols distinguished Architecture: Efficiency and maintainability tradeoff Encapsulation and cohesion Global and shared data minimized Entry and exit points minimized Non-standard or non-portable techniques avoided, or documented when used KISS Reasonable function length Reasonable nesting depth Exception conditions pre-empted or handled No redundant code No dead code All build warnings eliminated or addressed
-
I have been asked to work on our Code Review Procedure. This sort of thing: Pascal casing used for type and method names Camel casing used for local variable names and method arguments ... A single file should only contribute types to a single namespace. Avoid having multiple namespaces in the same file. Avoid files with more than 500 lines (excluding machine-generated code). Avoid methods with more than 25 lines. ... Never hard-code a numeric value, always declare a constant instead. ... I have the feeling it is missing what really make maintenance go bad. Things like re use existing classes, don't duplicate code, separate logic, data access, and display. Can anyone point me to some documents with good recommendations of this type. The idea is to minimize defect and certainly not to alienate the programmers (including me).
I had to do this at my last gig, only the shop had a very ingrown and long-established culture..."the design is in the code", "if the code is right, no one has to read it again", etc.. The approach I took which was at least moderately successful, was to first take the MS standards as the starting point -- they are pretty good and more importantly they set the tone for what is important and what is not. Stay away from the silly stuff like cunting comments or the number of spaces inside of comments and concentrate on what makes reliable, reusable, and readable. The second and more important aspect was to get the affected people involved early in a series of ongoing review/feedback opportunities so everyone felt like they had a chance to contribute. These don't have to be meetings - just post the in-progress standards somewhere and let people know they are available and you are taking comments. Those that are truly interested will participate, those that aren't ... they missed their chance. Also use 'guest authors' for specialty areas like .MSIs and SPROCs -- they know what is really important in their areas of specialty. We wound up with a set of standards and 'advisory' items which everyone pretty much adhered to, and that required a minimal amount of maintenance/updating on my part as MS went through its regular convulsions (er, new releases). Finally, make it clear that you have management support and that adherence to the final standards will be part of automated code reviews and individual performance reviews (you did get management buy-in at the beginning, right?). And of course, don't forget your body armor :suss:
“The trouble ain't that there is too many fools, but that the lightning ain't distributed right.” --Mark Twain
-
The problem is when different people edit the same files using varying setups - some using tabs, some using spaces, plus varying tab interpretations (4, 3, 8, etc ...). The end result is an indentation nightmare that looks as if a monkey has been dancing on your keyboard.
This looks like someone making his jobs easier, by making everyone else job harder. As I see it, as long as all the people on a project agree to a tab size there is no problem. And using spaces instead of tabs carry the problem that you will need to press the keyboard 4 to 8 times(depending on tab size) to indent and un-indent your code when with a tab you press only once. When in doubt, just put a comment line telling whoever wants to maintain your code your tab size.
-
You can just configure the IDE to replace tabs with spaces whenever you type them... Then again, it can do most of the indenting on its own too!
Yep, kind of my point. who needs to really worry about spaces when the IDE will fill them in for you. I'd rather be concerned about thought process while coding than to worry about how many spaces I've put in.
"The clue train passed his station without stopping." - John Simmons / outlaw programmer "Real programmers just throw a bunch of 1s and 0s at the computer to see what sticks" - Pete O'Hanlon "Not only do you continue to babble nonsense, you can't even correctly remember the nonsense you babbled just minutes ago." - Rob Graham
-
This looks like someone making his jobs easier, by making everyone else job harder. As I see it, as long as all the people on a project agree to a tab size there is no problem. And using spaces instead of tabs carry the problem that you will need to press the keyboard 4 to 8 times(depending on tab size) to indent and un-indent your code when with a tab you press only once. When in doubt, just put a comment line telling whoever wants to maintain your code your tab size.
rxantos wrote:
And using spaces instead of tabs carry the problem that you will need to press the keyboard 4 to 8 times
IDE's allow you to press TAB to insert spaces and SHIFT+TAB to unindent. It's just a matter of setting the correct option. You do not need to press the space key.